Merge pull request #311 from ethereum/improveStaticAnalysis

Static analysis: Display warning for each reported item

src/app.js

@@ -20,6 +20,7 @@ var Debugger = require('./app/debugger')
 var FormalVerification = require('./app/formalVerification')
 var EventManager = require('./lib/eventManager')
 var StaticAnalysis = require('./app/staticanalysis/staticAnalysisView')
+var OffsetToLineColumnConverter = require('./lib/offsetToLineColumnConverter')
 
 // The event listener needs to be registered as early as possible, because the
 // parent will send the message upon the "load" event.
@@ -423,12 +424,13 @@ var run = function () {
         cb(err || 'Unknown transport error')
       })
   }
-
   var executionContext = new ExecutionContext()
   var compiler = new Compiler(editor, handleGithubCall)
   var formalVerification = new FormalVerification($('#verificationView'), compiler.event)
 
-  var transactionDebugger = new Debugger('#debugger', editor, compiler, executionContext.event, swicthToFile)
+  var offsetToLineColumnConverter = new OffsetToLineColumnConverter(compiler.event)
+
+  var transactionDebugger = new Debugger('#debugger', editor, compiler, executionContext.event, swicthToFile, offsetToLineColumnConverter)
   transactionDebugger.addProvider('vm', executionContext.vm())
   transactionDebugger.switchProvider('vm')
   transactionDebugger.addProvider('injected', executionContext.web3())
@@ -445,7 +447,7 @@ var run = function () {
 
   var renderer = new Renderer(editor, executionContext.web3(), updateFiles, udapp, executionContext, formalVerification.event, compiler.event) // eslint-disable-line
 
-  var staticanalysis = new StaticAnalysis(compiler, renderer)
+  var staticanalysis = new StaticAnalysis(compiler.event, renderer, editor, offsetToLineColumnConverter)
   $('#staticanalysisView').append(staticanalysis.render())
 
   var autoCompile = document.querySelector('#autoCompile').checked

src/app/debugger.js

@@ -8,15 +8,15 @@ var Range = ace.acequire('ace/range').Range
 /**
  * Manage remix and source highlighting
  */
-function Debugger (id, editor, compiler, executionContextEvent, switchToFile) {
+function Debugger (id, editor, compiler, executionContextEvent, switchToFile, offsetToLineColumnConverter) {
   this.el = document.querySelector(id)
+  this.offsetToLineColumnConverter = offsetToLineColumnConverter
   this.debugger = new remix.ui.Debugger()
   this.sourceMappingDecoder = new remix.util.SourceMappingDecoder()
   this.el.appendChild(this.debugger.render())
   this.editor = editor
   this.switchToFile = switchToFile
   this.compiler = compiler
-  this.cache = new Cache()
 
   var self = this
   executionContextEvent.register('contextChanged', this, function (context) {
@@ -25,13 +25,11 @@ function Debugger (id, editor, compiler, executionContextEvent, switchToFile) {
 
   this.lastCompilationResult = null
   this.debugger.event.register('newTraceLoaded', this, function () {
-    self.cache.clear()
     self.lastCompilationResult = self.compiler.lastCompilationResult
   })
 
   this.debugger.event.register('traceUnloaded', this, function () {
     self.removeCurrentMarker()
-    self.cache.clear()
   })
 
   this.editor.onChangeSetup(function () {
@@ -45,10 +43,7 @@ function Debugger (id, editor, compiler, executionContextEvent, switchToFile) {
     if (self.lastCompilationResult) {
       this.debugger.sourceLocationTracker.getSourceLocation(address, index, self.lastCompilationResult.data.contracts, function (error, rawLocation) {
         if (!error) {
-          if (!self.cache.lineBreakPositionsByContent[address]) {
-            self.cache.lineBreakPositionsByContent[address] = self.sourceMappingDecoder.getLinebreakPositions(self.editor.getFile(self.lastCompilationResult.data.sourceList[rawLocation.file]))
-          }
-          var lineColumnPos = self.sourceMappingDecoder.convertOffsetToLineColumn(rawLocation, self.cache.lineBreakPositionsByContent[address])
+          var lineColumnPos = self.offsetToLineColumnConverter.offsetToLineColumn(rawLocation, rawLocation.file, self.editor, self.lastCompilationResult)
           self.highlight(lineColumnPos, rawLocation)
         } else {
           self.removeCurrentMarker()
@@ -125,12 +120,4 @@ Debugger.prototype.removeCurrentMarker = function () {
   }
 }
 
-function Cache () {
-  this.contentLineBreakPosition = {}
-}
-
-Cache.prototype.clear = function () {
-  this.lineBreakPositionsByContent = {}
-}
-
 module.exports = Debugger

src/app/staticanalysis/modules/txOrigin.js

@@ -17,9 +17,12 @@ txOrigin.prototype.visit = function (node) {
 }
 
 txOrigin.prototype.report = function (node) {
-  var report = this.txOriginNode.length + ' use of tx.origin\n'
+  var report = []
   this.txOriginNode.map(function (item, i) {
-    report += item.src + '\n'
+    report.push({
+      warning: 'use of tx.origin: "tx.origin" is useful only in very exceptional cases.\nIf you use it for authentication, you usually want to replace it by "msg.sender", because otherwise any contract you call can act on your behalf.',
+      location: item.src
+    })
   })
   return {
     name: name,

src/app/staticanalysis/staticAnalysisView.js

@@ -3,17 +3,20 @@ var StaticAnalysisRunner = require('./staticAnalysisRunner.js')
 var yo = require('yo-yo')
 var $ = require('jquery')
 
-function staticAnalysisView (compiler, renderer) {
+function staticAnalysisView (compilerEvent, renderer, editor, offsetToColumnConverter) {
   this.view = null
   this.renderer = renderer
+  this.editor = editor
   this.runner = new StaticAnalysisRunner()
+  this.offsetToColumnConverter = offsetToColumnConverter
   this.modulesView = renderModules(this.runner.modules())
-  this.lastASTs = null
+  this.lastCompilationResult = null
   var self = this
-  compiler.event.register('compilationFinished', function (success, data, source) {
-    self.lastASTs = null
+  compilerEvent.register('compilationFinished', function (success, data, source) {
+    self.lastCompilationResult = null
+    $('#staticanalysisresult').empty()
     if (success) {
-      self.lastASTs = data.sources
+      self.lastCompilationResult = data
     }
   })
 }
@@ -58,11 +61,21 @@ staticAnalysisView.prototype.run = function () {
   var selected = this.selectedModules()
   var warningContainer = $('#staticanalysisresult')
   warningContainer.empty()
-  if (this.lastASTs) {
+  if (this.lastCompilationResult) {
     var self = this
-    this.runner.run(this.lastASTs, selected, function (results) {
-      results.map(function (item, i) {
-        self.renderer.error(item.name + ':\n\n' + item.report, warningContainer, null, 'warning')
+    this.runner.run(this.lastCompilationResult.sources, selected, function (results) {
+      results.map(function (result, i) {
+        result.report.map(function (item, i) {
+          var split = item.location.split(':')
+          var file = split[2]
+          var location = {
+            start: parseInt(split[0]),
+            length: parseInt(split[1])
+          }
+          location = self.offsetToColumnConverter.offsetToLineColumn(location, file, self.editor, self.lastCompilationResult)
+          location = self.lastCompilationResult.sourceList[file] + ':' + (location.start.line + 1) + ':' + (location.start.column + 1) + ':'
+          self.renderer.error(location + ' ' + item.warning, warningContainer, false, 'warning')
+        })
       })
     })
   } else {

src/lib/offsetToLineColumnConverter.js

@@ -0,0 +1,24 @@
+'use strict'
+var SourceMappingDecoder = require('ethereum-remix').util.SourceMappingDecoder
+
+function offsetToColumnConverter (compilerEvent) {
+  this.lineBreakPositionsByContent = {}
+  this.sourceMappingDecoder = new SourceMappingDecoder()
+  var self = this
+  compilerEvent.register('compilationFinished', function (success, data, source) {
+    self.clear()
+  })
+}
+
+offsetToColumnConverter.prototype.offsetToLineColumn = function (rawLocation, file, editor, compilationResult) {
+  if (!this.lineBreakPositionsByContent[file]) {
+    this.lineBreakPositionsByContent[file] = this.sourceMappingDecoder.getLinebreakPositions(editor.getFile(compilationResult.sourceList[file]))
+  }
+  return this.sourceMappingDecoder.convertOffsetToLineColumn(rawLocation, this.lineBreakPositionsByContent[file])
+}
+
+offsetToColumnConverter.prototype.clear = function () {
+  this.lineBreakPositionsByContent = {}
+}
+
+module.exports = offsetToColumnConverter

test-browser/tests/staticanalysis.js

@@ -30,7 +30,7 @@ function runTests (browser) {
     .click('.staticanalysisView')
     .click('#staticanalysisView button')
     .waitForElementPresent('#staticanalysisresult .warning')
-    .assert.containsText('#staticanalysisresult .warning pre', '1 use of tx.origin')
+    .assert.containsText('#staticanalysisresult .warning pre', 'Untitled:1:33: use of tx.origin')
     .end()
   })
 }