Merge branch 'master' of https://github.com/ethereum/remix-project into react-plugin-manager

apps/remix-ide-e2e/src/tests/remixd.test.ts

@@ -34,6 +34,13 @@ const sources = [
   },
   {
     'test_import_node_modules_with_github_import.sol': { content: 'import "openzeppelin-solidity/contracts/sample.sol";' }
+  },
+  {
+    'test_static_analysis_with_remixd_and_hardhat.sol': {
+      content: `
+      import "hardhat/console.sol";
+      contract test5 { function get () public returns (uint) { return 8; }}`
+    }
   }
 ]
 
@@ -71,6 +78,21 @@ module.exports = {
       .setSolidityCompilerVersion('soljson-v0.8.0+commit.c7dfd78e.js') // open-zeppelin moved to pragma ^0.8.0
       .testContracts('test_import_node_modules_with_github_import.sol', sources[4]['test_import_node_modules_with_github_import.sol'], ['ERC20', 'test11'])
   },
+  'Static Analysis run with remixd': function (browser) {
+    browser.testContracts('test_static_analysis_with_remixd_and_hardhat.sol', sources[5]['test_static_analysis_with_remixd_and_hardhat.sol'], ['test5'])
+      .clickLaunchIcon('solidityStaticAnalysis')
+      .click('#staticanalysisButton button')
+      .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () {
+        browser
+          .click('[data-id="staticAnalysisModuleMiscellaneous1"')
+          .waitForElementPresent('.highlightLine15', 60000)
+          .getEditorValue((content) => {
+            browser.assert.ok(content.indexOf(
+              'function _sendLogPayload(bytes memory payload) private view {') !== -1,
+            'code has not been loaded')
+          })
+      })
+  },
 
   'Run git status': '' + function (browser) {
     browser

apps/remix-ide/src/app/files/fileManager.js

@@ -22,7 +22,7 @@ const profile = {
   icon: 'assets/img/fileManager.webp',
   permission: true,
   version: packageJson.version,
-  methods: ['file', 'exists', 'open', 'writeFile', 'readFile', 'copyFile', 'copyDir', 'rename', 'mkdir', 'readdir', 'remove', 'getCurrentFile', 'getFile', 'getFolder', 'setFile', 'switchFile', 'refresh', 'getProviderOf', 'getProviderByName'],
+  methods: ['file', 'exists', 'open', 'writeFile', 'readFile', 'copyFile', 'copyDir', 'rename', 'mkdir', 'readdir', 'remove', 'getCurrentFile', 'getFile', 'getFolder', 'setFile', 'switchFile', 'refresh', 'getProviderOf', 'getProviderByName', 'getPathFromUrl', 'getUrlFromPath'],
   kind: 'file-system'
 }
 const errorMsg = {
@@ -168,14 +168,11 @@ class FileManager extends Plugin {
    * @returns {void}
    */
   async open (path) {
-    try {
-      path = this.limitPluginScope(path)
-      await this._handleExists(path, `Cannot open file ${path}`)
-      await this._handleIsFile(path, `Cannot open file ${path}`)
-      return this.openFile(path)
-    } catch (e) {
-      throw new Error(e)
-    }
+    path = this.limitPluginScope(path)
+    path = this.getPathFromUrl(path).file
+    await this._handleExists(path, `Cannot open file ${path}`)
+    await this._handleIsFile(path, `Cannot open file ${path}`)
+    await this.openFile(path)
   }
 
   /**
@@ -538,6 +535,36 @@ class FileManager extends Plugin {
     }
   }
 
+  /**
+   * Try to resolve the given file path (the actual path in the file system)
+   * e.g if it's specified a github link, npm library, or any external content,
+   * it returns the actual path where the content can be found.
+   * @param {string} file url we are trying to resolve
+   * @returns {{ string, provider }} file path resolved and its provider.
+   */
+  getPathFromUrl (file) {
+    const provider = this.fileProviderOf(file)
+    if (!provider) throw new Error(`no provider for ${file}`)
+    return {
+      file: provider.getPathFromUrl(file) || file, // in case an external URL is given as input, we resolve it to the right internal path
+      provider
+    }
+  }
+
+  /**
+   * Try to resolve the given file URl. opposite of getPathFromUrl
+   * @param {string} file path we are trying to resolve
+   * @returns {{ string, provider }} file url resolved and its provider.
+   */
+  getUrlFromPath (file) {
+    const provider = this.fileProviderOf(file)
+    if (!provider) throw new Error(`no provider for ${file}`)
+    return {
+      file: provider.getUrlFromPath(file) || file, // in case an external URL is given as input, we resolve it to the right internal path
+      provider
+    }
+  }
+
   removeTabsOf (provider) {
     for (var tab in this.openedFiles) {
       if (this.fileProviderOf(tab).type === provider.type) {
@@ -566,33 +593,37 @@ class FileManager extends Plugin {
     this.events.emit('noFileSelected')
   }
 
-  openFile (file) {
-    const _openFile = (file) => {
+  async openFile (file) {
+    if (!file) {
+      this.emit('noFileSelected')
+      this.events.emit('noFileSelected')
+    } else {
       this.saveCurrentFile()
-      const provider = this.fileProviderOf(file)
-      if (!provider) return console.error(`no provider for ${file}`)
-      file = provider.getPathFromUrl(file) || file // in case an external URL is given as input, we resolve it to the right internal path
+      const resolved = this.getPathFromUrl(file)
+      file = resolved.file
+      const provider = resolved.provider
       this._deps.config.set('currentFile', file)
       this.openedFiles[file] = file
-      provider.get(file, (error, content) => {
-        if (error) {
-          console.log(error)
-        } else {
-          if (provider.isReadOnly(file)) {
-            this.editor.openReadOnly(file, content)
-          } else {
-            this.editor.open(file, content)
-          }
-          // TODO: Only keep `this.emit` (issue#2210)
-          this.emit('currentFileChanged', file)
-          this.events.emit('currentFileChanged', file)
-        }
-      })
-    }
-    if (file) return _openFile(file)
-    else {
-      this.emit('noFileSelected')
-      this.events.emit('noFileSelected')
+      await (() => {
+        return new Promise((resolve, reject) => {
+          provider.get(file, (error, content) => {
+            if (error) {
+              console.log(error)
+              reject(error)
+            } else {
+              if (provider.isReadOnly(file)) {
+                this.editor.openReadOnly(file, content)
+              } else {
+                this.editor.open(file, content)
+              }
+              // TODO: Only keep `this.emit` (issue#2210)
+              this.emit('currentFileChanged', file)
+              this.events.emit('currentFileChanged', file)
+              resolve()
+            }
+          })
+        })
+      })()
     }
   }
 

apps/remix-ide/src/app/files/fileProvider.js

@@ -13,17 +13,18 @@ class FileProvider {
     this.type = name
     this.providerExternalsStorage = new Storage('providerExternals:')
     this.externalFolders = [this.type + '/swarm', this.type + '/ipfs', this.type + '/github', this.type + '/gists', this.type + '/https']
+    this.reverseKey = this.type + '-reverse-'
   }
 
   addNormalizedName (path, url) {
     this.providerExternalsStorage.set(this.type + '/' + path, url)
-    this.providerExternalsStorage.set('reverse-' + url, this.type + '/' + path)
+    this.providerExternalsStorage.set(this.reverseKey + url, this.type + '/' + path)
   }
 
   removeNormalizedName (path) {
     const value = this.providerExternalsStorage.get(path)
     this.providerExternalsStorage.remove(path)
-    this.providerExternalsStorage.remove('reverse-' + value)
+    this.providerExternalsStorage.remove(this.reverseKey + value)
   }
 
   normalizedNameExists (path) {
@@ -35,7 +36,12 @@ class FileProvider {
   }
 
   getPathFromUrl (url) {
-    return this.providerExternalsStorage.get('reverse-' + url)
+    return this.providerExternalsStorage.get(this.reverseKey + url)
+  }
+
+  getUrlFromPath (path) {
+    if (!path.startsWith(this.type)) path = this.type + '/' + path
+    return this.providerExternalsStorage.get(path)
   }
 
   isExternalFolder (path) {

apps/remix-ide/src/app/files/remixDProvider.js

@@ -87,14 +87,6 @@ module.exports = class RemixDProvider extends FileProvider {
       })
   }
 
-  getNormalizedName (path) {
-    return path
-  }
-
-  getPathFromUrl (path) {
-    return path
-  }
-
   get (path, cb) {
     if (!this._isReady) return cb && cb('provider not ready')
     var unprefixedpath = this.removePrefix(path)

apps/remix-ide/team-best-practices.md

@@ -177,7 +177,7 @@ Before starting coding, we should ensure all devs / contributors are aware of:
 
 # Coding best practices
 
- - https://github.com/ethereum/remix-ide/blob/master/best-practices.md
+ - https://github.com/ethereum/remix-project/blob/master/best-practices.md
 
 ---
 

libs/remix-core-plugin/src/lib/compiler-content-imports.ts

@@ -2,7 +2,6 @@
 import { Plugin } from '@remixproject/engine'
 import { RemixURLResolver } from '@remix-project/remix-url-resolver'
 const remixTests = require('@remix-project/remix-tests')
-const async = require('async')
 
 const profile = {
   name: 'contentImport',
@@ -88,21 +87,23 @@ export class CompilerImports extends Plugin {
     }
   }
 
-  importExternal (url, targetPath, cb) {
-    this.import(url,
-      // TODO: handle this event
-      (loadingMsg) => { this.emit('message', loadingMsg) },
-      async (error, content, cleanUrl, type, url) => {
-        if (error) return cb(error)
-        try {
-          const provider = await this.call('fileManager', 'getProviderOf', null)
-          const path = targetPath || type + '/' + cleanUrl
-          if (provider) provider.addExternal('.deps/' + path, content, url)
-        } catch (err) {
-
-        }
-        cb(null, content)
-      }, null)
+  importExternal (url, targetPath) {
+    return new Promise((resolve, reject) => {
+      this.import(url,
+        // TODO: handle this event
+        (loadingMsg) => { this.emit('message', loadingMsg) },
+        async (error, content, cleanUrl, type, url) => {
+          if (error) return reject(error)
+          try {
+            const provider = await this.call('fileManager', 'getProviderOf', null)
+            const path = targetPath || type + '/' + cleanUrl
+            if (provider) provider.addExternal('.deps/' + path, content, url)
+          } catch (err) {
+            console.error(err)
+          }
+          resolve(content)
+        }, null)
+    })
   }
 
   /**
@@ -115,66 +116,58 @@ export class CompilerImports extends Plugin {
     * @param {String} targetPath - (optional) internal path where the content should be saved to
     * @returns {Promise} - string content
     */
-  resolveAndSave (url, targetPath) {
-    return new Promise((resolve, reject) => {
-      if (url.indexOf('remix_tests.sol') !== -1) resolve(remixTests.assertLibCode)
-      this.call('fileManager', 'getProviderOf', url).then((provider) => {
-        if (provider) {
-          if (provider.type === 'localhost' && !provider.isConnected()) {
-            return reject(new Error(`file provider ${provider.type} not available while trying to resolve ${url}`))
-          }
-          provider.exists(url).then(exist => {
-            /*
-              if the path is absolute and the file does not exist, we can stop here
-              Doesn't make sense to try to resolve "localhost/node_modules/localhost/node_modules/<path>" and we'll end in an infinite loop.
-            */
-            if (!exist && url.startsWith('browser/')) return reject(new Error(`not found ${url}`))
-            if (!exist && url.startsWith('localhost/')) return reject(new Error(`not found ${url}`))
-
-            if (exist) {
-              return provider.get(url, (error, content) => {
-                if (error) return reject(error)
-                resolve(content)
-              })
-            }
+  async resolveAndSave (url, targetPath) {
+    if (url.indexOf('remix_tests.sol') !== -1) return remixTests.assertLibCode
+    try {
+      const provider = await this.call('fileManager', 'getProviderOf', url)
+      if (provider) {
+        if (provider.type === 'localhost' && !provider.isConnected()) {
+          throw new Error(`file provider ${provider.type} not available while trying to resolve ${url}`)
+        }
+        const exist = await provider.exists(url)
+        /*
+          if the path is absolute and the file does not exist, we can stop here
+          Doesn't make sense to try to resolve "localhost/node_modules/localhost/node_modules/<path>" and we'll end in an infinite loop.
+        */
+        if (!exist && url.startsWith('browser/')) throw new Error(`not found ${url}`)
+        if (!exist && url.startsWith('localhost/')) throw new Error(`not found ${url}`)
 
-            // try to resolve localhost modules (aka truffle imports) - e.g from the node_modules folder
-            this.call('fileManager', 'getProviderByName', 'localhost').then((localhostProvider) => {
-              if (localhostProvider.isConnected()) {
-                var splitted = /([^/]+)\/(.*)$/g.exec(url)
-                return async.tryEach([
-                  (cb) => { this.resolveAndSave('localhost/installed_contracts/' + url, null).then((result) => cb(null, result)).catch((error) => cb(error.message)) },
-                  // eslint-disable-next-line standard/no-callback-literal
-                  (cb) => { if (!splitted) { cb('URL not parseable: ' + url) } else { this.resolveAndSave('localhost/installed_contracts/' + splitted[1] + '/contracts/' + splitted[2], null).then((result) => cb(null, result)).catch((error) => cb(error.message)) } },
-                  (cb) => { this.resolveAndSave('localhost/node_modules/' + url, null).then((result) => cb(null, result)).catch((error) => cb(error.message)) },
-                  // eslint-disable-next-line standard/no-callback-literal
-                  (cb) => { if (!splitted) { cb('URL not parseable: ' + url) } else { this.resolveAndSave('localhost/node_modules/' + splitted[1] + '/contracts/' + splitted[2], null).then((result) => cb(null, result)).catch((error) => cb(error.message)) } }],
-                (error, result) => {
-                  if (error) {
-                    return this.importExternal(url, targetPath, (error, content) => {
-                      if (error) return reject(error)
-                      resolve(content)
-                    })
-                  }
-                  resolve(result)
-                })
-              }
-              this.importExternal(url, targetPath, (error, content) => {
+        if (exist) {
+          const content = await (() => {
+            return new Promise((resolve, reject) => {
+              provider.get(url, (error, content) => {
                 if (error) return reject(error)
                 resolve(content)
               })
             })
-          }).catch(error => {
-            return reject(error)
-          })
+          })()
+          return content
+        } else {
+          const localhostProvider = await this.call('fileManager', 'getProviderByName', 'localhost')
+          if (localhostProvider.isConnected()) {
+            const splitted = /([^/]+)\/(.*)$/g.exec(url)
+
+            const possiblePaths = ['localhost/installed_contracts/' + url]
+            if (splitted) possiblePaths.push('localhost/installed_contracts/' + splitted[1] + '/contracts/' + splitted[2])
+            possiblePaths.push('localhost/node_modules/' + url)
+            if (splitted) possiblePaths.push('localhost/node_modules/' + splitted[1] + '/contracts/' + splitted[2])
+
+            for (const path of possiblePaths) {
+              try {
+                const content = await this.resolveAndSave(path, null)
+                if (content) {
+                  localhostProvider.addNormalizedName(path.replace('localhost/', ''), url)
+                  return content
+                }
+              } catch (e) {}
+            }
+            return await this.importExternal(url, targetPath)
+          }
+          return await this.importExternal(url, targetPath)
         }
-      }).catch(() => {
-        // fallback to just resolving the file, it won't be saved in file manager
-        return this.importExternal(url, targetPath, (error, content) => {
-          if (error) return reject(error)
-          resolve(content)
-        })
-      })
-    })
+      }
+    } catch (e) {
+      throw new Error(`not found ${url}`)
+    }
   }
 }

libs/remix-ui/renderer/src/lib/renderer.tsx

@@ -29,20 +29,13 @@ export const Renderer = ({ message, opt = {}, plugin }: RendererProps) => {
     // ^ e.g:
     // browser/gm.sol: Warning: Source file does not specify required compiler version! Consider adding "pragma solidity ^0.6.12
     // https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v3.2.0/contracts/introspection/IERC1820Registry.sol:3:1: ParserError: Source file requires different compiler version (current compiler is 0.7.4+commit.3f05b770.Emscripten.clang) - note that nightly builds are considered to be strictly less than the released version
-    let positionDetails = getPositionDetails(text)
-    const options = opt
-
-    if (!positionDetails.errFile || (opt.errorType && opt.errorType === positionDetails.errFile)) {
-      // Updated error reported includes '-->' before file details
-      const errorDetails = text.split('-->')
-      // errorDetails[1] will have file details
-      if (errorDetails.length > 1) positionDetails = getPositionDetails(errorDetails[1])
-    }
-    options.errLine = positionDetails.errLine
-    options.errCol = positionDetails.errCol
-    options.errFile = positionDetails.errFile.trim()
+    const positionDetails = getPositionDetails(text)
+
+    opt.errLine = positionDetails.errLine
+    opt.errCol = positionDetails.errCol
+    opt.errFile = positionDetails.errFile ? (positionDetails.errFile as string).trim() : ''
 
-    if (!opt.noAnnotations && opt.errFile) {
+    if (!opt.noAnnotations && opt.errFile && opt.errFile !== '') {
       addAnnotation(opt.errFile, {
         row: opt.errLine,
         column: opt.errCol,
@@ -52,15 +45,17 @@ export const Renderer = ({ message, opt = {}, plugin }: RendererProps) => {
     }
 
     setMessageText(text)
-    setEditorOptions(options)
+    setEditorOptions(opt)
     setClose(false)
   }, [message, opt])
 
-  const getPositionDetails = (msg: any) => {
+  const getPositionDetails = (msg: string) => {
     const result = { } as Record<string, number | string>
 
     // To handle some compiler warning without location like SPDX license warning etc
-    if (!msg.includes(':')) return { errLine: -1, errCol: -1, errFile: msg }
+    if (!msg.includes(':')) return { errLine: -1, errCol: -1, errFile: '' }
+
+    if (msg.includes('-->')) msg = msg.split('-->')[1].trim()
 
     // extract line / column
     let pos = msg.match(/^(.*?):([0-9]*?):([0-9]*?)?/)
@@ -69,7 +64,7 @@ export const Renderer = ({ message, opt = {}, plugin }: RendererProps) => {
 
     // extract file
     pos = msg.match(/^(https:.*?|http:.*?|.*?):/)
-    result.errFile = pos ? pos[1] : ''
+    result.errFile = pos ? pos[1] : msg
     return result
   }
 

libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx

@@ -209,14 +209,14 @@ export const RemixUiStaticAnalyser = (props: RemixUiStaticAnalyserProps) => {
           })
           // Slither Analysis
           if (slitherEnabled) {
-            props.analysisModule.call('solidity-logic', 'getCompilerState').then((compilerState) => {
+            props.analysisModule.call('solidity-logic', 'getCompilerState').then(async (compilerState) => {
               const { currentVersion, optimize, evmVersion } = compilerState
               props.analysisModule.call('terminal', 'log', { type: 'info', value: '[Slither Analysis]: Running...' })
-              props.analysisModule.call('slither', 'analyse', state.file, { currentVersion, optimize, evmVersion }).then((result) => {
+              props.analysisModule.call('slither', 'analyse', state.file, { currentVersion, optimize, evmVersion }).then(async (result) => {
                 if (result.status) {
                   props.analysisModule.call('terminal', 'log', { type: 'info', value: `[Slither Analysis]: Analysis Completed!! ${result.count} warnings found.` })
                   const report = result.data
-                  report.map((item) => {
+                  for (const item of report) {
                     let location: any = {}
                     let locationString = 'not available'
                     let column = 0
@@ -224,7 +224,12 @@ export const RemixUiStaticAnalyser = (props: RemixUiStaticAnalyserProps) => {
                     let fileName = currentFile
 
                     if (item.sourceMap && item.sourceMap.length) {
-                      const fileIndex = Object.keys(lastCompilationResult.sources).indexOf(item.sourceMap[0].source_mapping.filename_relative)
+                      let path = item.sourceMap[0].source_mapping.filename_relative
+                      let fileIndex = Object.keys(lastCompilationResult.sources).indexOf(path)
+                      if (fileIndex === -1) {
+                        path = await props.analysisModule.call('fileManager', 'getUrlFromPath', path)
+                        fileIndex = Object.keys(lastCompilationResult.sources).indexOf(path.file)
+                      }
                       if (fileIndex >= 0) {
                         location = {
                           start: item.sourceMap[0].source_mapping.start,
@@ -259,7 +264,7 @@ export const RemixUiStaticAnalyser = (props: RemixUiStaticAnalyserProps) => {
                     }
                     warningErrors.push(options)
                     warningMessage.push({ msg, options, hasWarning: true, warningModuleName: 'Slither Analysis' })
-                  })
+                  }
                   showWarnings(warningMessage, 'warningModuleName')
                   props.event.trigger('staticAnaysisWarning', [warningCount])
                 }
@@ -466,7 +471,7 @@ export const RemixUiStaticAnalyser = (props: RemixUiStaticAnalyserProps) => {
                   <span className="text-dark h6">{element[0]}</span>
                   {element[1]['map']((x, i) => ( // eslint-disable-line dot-notation
                     x.hasWarning ? ( // eslint-disable-next-line  dot-notation
-                      <div id={`staticAnalysisModule${element[1]['warningModuleName']}`} key={i}>
+                      <div data-id={`staticAnalysisModule${x.warningModuleName}${i}`} id={`staticAnalysisModule${x.warningModuleName}${i}`} key={i}>
                         <ErrorRenderer message={x.msg} opt={x.options} warningErrors={ x.warningErrors} editor={props.analysisModule}/>
                       </div>
 

team-best-practices.md

@@ -189,4 +189,4 @@ Before starting coding, we should ensure all devs / contributors are aware of:
 
 # Coding best practices
 
- - https://github.com/ethereum/remix-ide/blob/master/best-practices.md
+ - https://github.com/ethereum/remix-project/blob/master/best-practices.md