From b8cc5423519e9b0d5d4dbcc31c990c8350de2ddd Mon Sep 17 00:00:00 2001 From: yann300 Date: Mon, 23 Aug 2021 12:58:26 +0200 Subject: [PATCH 01/20] remove specific implementation, that way the base "FileProvider" implementation is used --- apps/remix-ide/src/app/files/remixDProvider.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/apps/remix-ide/src/app/files/remixDProvider.js b/apps/remix-ide/src/app/files/remixDProvider.js index 04b0217df1..1a9d68dff7 100644 --- a/apps/remix-ide/src/app/files/remixDProvider.js +++ b/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) From bed8feff4d12536f26959ce021b1e6625fb83adc Mon Sep 17 00:00:00 2001 From: yann300 Date: Mon, 23 Aug 2021 12:59:46 +0200 Subject: [PATCH 02/20] make sure the reverse key contains the provider type, so both provider (localhost/browser) doesn't share the same normalized names --- apps/remix-ide/src/app/files/fileProvider.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/remix-ide/src/app/files/fileProvider.js b/apps/remix-ide/src/app/files/fileProvider.js index c789635160..57e21d2eab 100644 --- a/apps/remix-ide/src/app/files/fileProvider.js +++ b/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,7 @@ class FileProvider { } getPathFromUrl (url) { - return this.providerExternalsStorage.get('reverse-' + url) + return this.providerExternalsStorage.get(this.reverseKey + url) } isExternalFolder (path) { From 9f9d1f4df586295e9fe792f817ca5e75871a8309 Mon Sep 17 00:00:00 2001 From: yann300 Date: Mon, 23 Aug 2021 13:00:33 +0200 Subject: [PATCH 03/20] resolving external path for the "open" function was missing --- apps/remix-ide/src/app/files/fileManager.js | 32 +++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index f2d83d3915..1da55cf8c4 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/apps/remix-ide/src/app/files/fileManager.js @@ -170,6 +170,11 @@ class FileManager extends Plugin { async open (path) { try { path = this.limitPluginScope(path) + try { + path = this._resolveFromExternalPath(path).file || path + } catch (e) { + return console.error(e) + } await this._handleExists(path, `Cannot open file ${path}`) await this._handleIsFile(path, `Cannot open file ${path}`) return this.openFile(path) @@ -538,6 +543,22 @@ class FileManager extends Plugin { } } + /** + * Try to resolve the given file path. + * 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 find. + * @param {string} file path we are trying to resolve + * @returns {{ string, provider }} file path resolved and its provider. + */ + _resolveFromExternalPath (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 + } + } + removeTabsOf (provider) { for (var tab in this.openedFiles) { if (this.fileProviderOf(tab).type === provider.type) { @@ -569,9 +590,14 @@ class FileManager extends Plugin { openFile (file) { const _openFile = (file) => { 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 + let resolved + try { + resolved = this._resolveFromExternalPath(file) + file = resolved.file + } catch (e) { + return console.error(e) + } + const provider = resolved.provider this._deps.config.set('currentFile', file) this.openedFiles[file] = file provider.get(file, (error, content) => { From 3273dfdf0c62ca7152175518ece46252a3fff295 Mon Sep 17 00:00:00 2001 From: yann300 Date: Mon, 23 Aug 2021 13:22:50 +0200 Subject: [PATCH 04/20] make resolveAndSave async and make sure node_modules path import are normalized --- .../src/lib/compiler-content-imports.ts | 134 +++++++++--------- 1 file changed, 64 insertions(+), 70 deletions(-) diff --git a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts b/libs/remix-core-plugin/src/lib/compiler-content-imports.ts index edb512a7ed..3f17feec6b 100644 --- a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts +++ b/libs/remix-core-plugin/src/lib/compiler-content-imports.ts @@ -88,21 +88,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 +117,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/" 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}`)) + 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/" 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}`) - if (exist) { - return provider.get(url, (error, content) => { + if (exist) { + const content = await (() => { + return new Promise((resolve, reject) => { + provider.get(url, (error, content) => { if (error) return reject(error) resolve(content) }) - } + }) + })() + return content + } else { + const localhostProvider = await this.call('fileManager', 'getProviderByName', 'localhost') + if (localhostProvider.isConnected()) { + var splitted = /([^/]+)\/(.*)$/g.exec(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 (error) return reject(error) - resolve(content) - }) - }) - }).catch(error => { - return reject(error) - }) + 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) { + return await this.importExternal(url, targetPath) + } } } From d68f7d11456fa642007bbf89979387da9ca6d963 Mon Sep 17 00:00:00 2001 From: yann300 Date: Mon, 23 Aug 2021 15:05:39 +0200 Subject: [PATCH 05/20] linting --- .../src/lib/compiler-content-imports.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts b/libs/remix-core-plugin/src/lib/compiler-content-imports.ts index 3f17feec6b..cf57e7cf2b 100644 --- a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts +++ b/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', @@ -104,7 +103,7 @@ export class CompilerImports extends Plugin { } resolve(content) }, null) - }) + }) } /** @@ -140,16 +139,16 @@ export class CompilerImports extends Plugin { if (error) return reject(error) resolve(content) }) - }) + }) })() - return content + return content } else { const localhostProvider = await this.call('fileManager', 'getProviderByName', 'localhost') if (localhostProvider.isConnected()) { var splitted = /([^/]+)\/(.*)$/g.exec(url) const possiblePaths = ['localhost/installed_contracts/' + url] - if (splitted) possiblePaths.push('localhost/installed_contracts/' + splitted[1] + '/contracts/' + splitted[2]) + 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]) From abcc5f0c0f15e931362882962a6058184a6ae7e1 Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 24 Aug 2021 12:37:41 +0200 Subject: [PATCH 06/20] fix and add getPathFromUrl and getUrlFromPath --- apps/remix-ide/src/app/files/fileManager.js | 28 +++++++++++++++----- apps/remix-ide/src/app/files/fileProvider.js | 5 ++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index 1da55cf8c4..4ef7b71587 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/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 = { @@ -171,7 +171,7 @@ class FileManager extends Plugin { try { path = this.limitPluginScope(path) try { - path = this._resolveFromExternalPath(path).file || path + path = this.getPathFromUrl(path).file } catch (e) { return console.error(e) } @@ -544,13 +544,13 @@ class FileManager extends Plugin { } /** - * Try to resolve the given file path. + * 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 find. - * @param {string} file path we are trying to resolve + * 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. */ - _resolveFromExternalPath (file) { + getPathFromUrl (file) { const provider = this.fileProviderOf(file) if (!provider) throw new Error(`no provider for ${file}`) return { @@ -559,6 +559,20 @@ class FileManager extends Plugin { } } + /** + * 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) { @@ -592,7 +606,7 @@ class FileManager extends Plugin { this.saveCurrentFile() let resolved try { - resolved = this._resolveFromExternalPath(file) + resolved = this.getPathFromUrl(file) file = resolved.file } catch (e) { return console.error(e) diff --git a/apps/remix-ide/src/app/files/fileProvider.js b/apps/remix-ide/src/app/files/fileProvider.js index 57e21d2eab..41e1a5ccfe 100644 --- a/apps/remix-ide/src/app/files/fileProvider.js +++ b/apps/remix-ide/src/app/files/fileProvider.js @@ -39,6 +39,11 @@ class FileProvider { 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) { return this.externalFolders.includes(path) } From 8a8b2331964dabc58dc2557219f8ef40320bea70 Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 24 Aug 2021 12:55:43 +0200 Subject: [PATCH 07/20] when resolving the file name, make sure we take care of aliases --- .../src/lib/remix-ui-static-analyser.tsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx b/libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx index 37211e5358..c16de17803 100644 --- a/libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx +++ b/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]) } From e873dac6af6ca6b462b482dfa1d41123e823f616 Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 24 Aug 2021 12:56:21 +0200 Subject: [PATCH 08/20] handle solidity error messages --- libs/remix-ui/renderer/src/lib/renderer.tsx | 26 +++++++++------------ 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/libs/remix-ui/renderer/src/lib/renderer.tsx b/libs/remix-ui/renderer/src/lib/renderer.tsx index 8b4799a4f0..306d850728 100644 --- a/libs/remix-ui/renderer/src/lib/renderer.tsx +++ b/libs/remix-ui/renderer/src/lib/renderer.tsx @@ -29,25 +29,19 @@ 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 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() + options.errFile = positionDetails.errFile ? (positionDetails.errFile as string).trim() : '' - if (!opt.noAnnotations && opt.errFile) { + if (!opt.noAnnotations && options.errFile && options.errFile !== '') { addAnnotation(opt.errFile, { - row: opt.errLine, - column: opt.errCol, + row: options.errLine, + column: options.errCol, text: text, - type: opt.type + type: options.type }) } @@ -56,11 +50,13 @@ export const Renderer = ({ message, opt = {}, plugin }: RendererProps) => { setClose(false) }, [message, opt]) - const getPositionDetails = (msg: any) => { + const getPositionDetails = (msg: string) => { const result = { } as Record // 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 +65,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 } From 91932c2aec618f48741079c266f943b5b7a7c97d Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 24 Aug 2021 14:13:33 +0200 Subject: [PATCH 09/20] e2e test: running static analysis with remixd and hardhat --- apps/remix-ide-e2e/src/tests/remixd.test.ts | 22 +++++++++++++++++++ .../src/lib/remix-ui-static-analyser.tsx | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/apps/remix-ide-e2e/src/tests/remixd.test.ts b/apps/remix-ide-e2e/src/tests/remixd.test.ts index 24a6ce9b96..e76561b20f 100644 --- a/apps/remix-ide-e2e/src/tests/remixd.test.ts +++ b/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('Untitled.sol', sources[5]['test_static_analysis_with_remixd_and_hardhat'], ['test5']) + .clickLaunchIcon('solidityStaticAnalysis') + .click('#staticanalysisButton button') + .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () { + browser + .click('[data-id="staticAnalysisModuleMiscellaneous1"') + .click('[data-id="staticAnalysisModuleMiscellaneous1"') + .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 diff --git a/libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx b/libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx index c16de17803..ace945e95e 100644 --- a/libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx +++ b/libs/remix-ui/static-analyser/src/lib/remix-ui-static-analyser.tsx @@ -471,7 +471,7 @@ export const RemixUiStaticAnalyser = (props: RemixUiStaticAnalyserProps) => { {element[0]} {element[1]['map']((x, i) => ( // eslint-disable-line dot-notation x.hasWarning ? ( // eslint-disable-next-line dot-notation -
+
From cc5378ba550aab0329a35e856648a4f4e803c0e3 Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 24 Aug 2021 16:10:19 +0200 Subject: [PATCH 10/20] linting --- apps/remix-ide-e2e/src/tests/remixd.test.ts | 23 ++++++++++---------- apps/remix-ide/src/app/files/fileManager.js | 12 +++++----- apps/remix-ide/src/app/files/fileProvider.js | 2 +- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/apps/remix-ide-e2e/src/tests/remixd.test.ts b/apps/remix-ide-e2e/src/tests/remixd.test.ts index e76561b20f..56b185d338 100644 --- a/apps/remix-ide-e2e/src/tests/remixd.test.ts +++ b/apps/remix-ide-e2e/src/tests/remixd.test.ts @@ -79,19 +79,18 @@ module.exports = { .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('Untitled.sol', sources[5]['test_static_analysis_with_remixd_and_hardhat'], ['test5']) - .clickLaunchIcon('solidityStaticAnalysis') - .click('#staticanalysisButton button') - .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () { - browser - .click('[data-id="staticAnalysisModuleMiscellaneous1"') - .click('[data-id="staticAnalysisModuleMiscellaneous1"') - .getEditorValue((content) => { - browser.assert.ok(content.indexOf( - 'function _sendLogPayload(bytes memory payload) private view {') !== -1, - 'code has not been loaded') + 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"') + .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) { diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index 4ef7b71587..68ac44199e 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/apps/remix-ide/src/app/files/fileManager.js @@ -545,29 +545,29 @@ 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, + * 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) { + getPathFromUrl (file) { const provider = this.fileProviderOf(file) if (!provider) throw new Error(`no provider for ${file}`) - return { + 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 * + * 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) { + getUrlFromPath (file) { const provider = this.fileProviderOf(file) if (!provider) throw new Error(`no provider for ${file}`) - return { + return { file: provider.getUrlFromPath(file) || file, // in case an external URL is given as input, we resolve it to the right internal path provider } diff --git a/apps/remix-ide/src/app/files/fileProvider.js b/apps/remix-ide/src/app/files/fileProvider.js index 41e1a5ccfe..04d055a649 100644 --- a/apps/remix-ide/src/app/files/fileProvider.js +++ b/apps/remix-ide/src/app/files/fileProvider.js @@ -40,7 +40,7 @@ class FileProvider { } getUrlFromPath (path) { - if (!path.startsWith(this.type)) path = this.type + '/' + path + if (!path.startsWith(this.type)) path = this.type + '/' + path return this.providerExternalsStorage.get(path) } From 2e6bb7e07f45e68389ea9ec0fe0982c154ba506d Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 10:44:25 +0200 Subject: [PATCH 11/20] simplify "openFile" and make it async --- apps/remix-ide/src/app/files/fileManager.js | 50 +++++++++++---------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index 68ac44199e..a5618995a9 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/apps/remix-ide/src/app/files/fileManager.js @@ -177,7 +177,7 @@ class FileManager extends Plugin { } await this._handleExists(path, `Cannot open file ${path}`) await this._handleIsFile(path, `Cannot open file ${path}`) - return this.openFile(path) + await this.openFile(path) } catch (e) { throw new Error(e) } @@ -601,8 +601,11 @@ 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() let resolved try { @@ -614,26 +617,27 @@ class FileManager extends Plugin { 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() + } + }) + }) + })() + } } /** From aa6919b7fc71e5bfff13eeedf9ee2c0c7f7a08ce Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 10:47:04 +0200 Subject: [PATCH 12/20] test if line is highlighted --- apps/remix-ide-e2e/src/tests/remixd.test.ts | 1 + apps/remix-ide/src/app/files/fileManager.js | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/remix-ide-e2e/src/tests/remixd.test.ts b/apps/remix-ide-e2e/src/tests/remixd.test.ts index 56b185d338..fcaaa167ee 100644 --- a/apps/remix-ide-e2e/src/tests/remixd.test.ts +++ b/apps/remix-ide-e2e/src/tests/remixd.test.ts @@ -85,6 +85,7 @@ module.exports = { .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () { browser .click('[data-id="staticAnalysisModuleMiscellaneous1"') + .waitForElementPresent('.highlightLine6', 60000) .getEditorValue((content) => { browser.assert.ok(content.indexOf( 'function _sendLogPayload(bytes memory payload) private view {') !== -1, diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index a5618995a9..959c1d75a7 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/apps/remix-ide/src/app/files/fileManager.js @@ -636,8 +636,8 @@ class FileManager extends Plugin { } }) }) - })() - } + })() + } } /** From d19d1d6ab11b6647c9aa1e2a423989178d190066 Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 11:16:01 +0200 Subject: [PATCH 13/20] linting --- libs/remix-core-plugin/src/lib/compiler-content-imports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts b/libs/remix-core-plugin/src/lib/compiler-content-imports.ts index cf57e7cf2b..5bc151c9b3 100644 --- a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts +++ b/libs/remix-core-plugin/src/lib/compiler-content-imports.ts @@ -145,7 +145,7 @@ export class CompilerImports extends Plugin { } else { const localhostProvider = await this.call('fileManager', 'getProviderByName', 'localhost') if (localhostProvider.isConnected()) { - var splitted = /([^/]+)\/(.*)$/g.exec(url) + const splitted = /([^/]+)\/(.*)$/g.exec(url) const possiblePaths = ['localhost/installed_contracts/' + url] if (splitted) possiblePaths.push('localhost/installed_contracts/' + splitted[1] + '/contracts/' + splitted[2]) From 9c59bcbafcdcd981ae12010cc65c6d805b7a876c Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 11:16:30 +0200 Subject: [PATCH 14/20] remove uneeded var assignement --- libs/remix-ui/renderer/src/lib/renderer.tsx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/libs/remix-ui/renderer/src/lib/renderer.tsx b/libs/remix-ui/renderer/src/lib/renderer.tsx index 306d850728..815a4c85ee 100644 --- a/libs/remix-ui/renderer/src/lib/renderer.tsx +++ b/libs/remix-ui/renderer/src/lib/renderer.tsx @@ -30,23 +30,22 @@ export const Renderer = ({ message, opt = {}, plugin }: RendererProps) => { // 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 const positionDetails = getPositionDetails(text) - const options = opt - options.errLine = positionDetails.errLine - options.errCol = positionDetails.errCol - options.errFile = positionDetails.errFile ? (positionDetails.errFile as string).trim() : '' + opt.errLine = positionDetails.errLine + opt.errCol = positionDetails.errCol + opt.errFile = positionDetails.errFile ? (positionDetails.errFile as string).trim() : '' - if (!opt.noAnnotations && options.errFile && options.errFile !== '') { + if (!opt.noAnnotations && opt.errFile && opt.errFile !== '') { addAnnotation(opt.errFile, { - row: options.errLine, - column: options.errCol, + row: opt.errLine, + column: opt.errCol, text: text, - type: options.type + type: opt.type }) } setMessageText(text) - setEditorOptions(options) + setEditorOptions(opt) setClose(false) }, [message, opt]) From 42e8192d60a97bb1ecefe6b923e1b4e55e441b6e Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 11:31:42 +0200 Subject: [PATCH 15/20] fix e2e --- apps/remix-ide-e2e/src/tests/remixd.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/remix-ide-e2e/src/tests/remixd.test.ts b/apps/remix-ide-e2e/src/tests/remixd.test.ts index fcaaa167ee..b62f0ed86e 100644 --- a/apps/remix-ide-e2e/src/tests/remixd.test.ts +++ b/apps/remix-ide-e2e/src/tests/remixd.test.ts @@ -85,7 +85,7 @@ module.exports = { .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () { browser .click('[data-id="staticAnalysisModuleMiscellaneous1"') - .waitForElementPresent('.highlightLine6', 60000) + .waitForElementPresent('.highlightLine15', 60000) .getEditorValue((content) => { browser.assert.ok(content.indexOf( 'function _sendLogPayload(bytes memory payload) private view {') !== -1, From 99ad36a299ca866f3a690b3ff836c69af3912586 Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 12:26:51 +0200 Subject: [PATCH 16/20] don't try to silly resolve if it catches --- libs/remix-core-plugin/src/lib/compiler-content-imports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts b/libs/remix-core-plugin/src/lib/compiler-content-imports.ts index 5bc151c9b3..2e20d5ebb7 100644 --- a/libs/remix-core-plugin/src/lib/compiler-content-imports.ts +++ b/libs/remix-core-plugin/src/lib/compiler-content-imports.ts @@ -167,7 +167,7 @@ export class CompilerImports extends Plugin { } } } catch (e) { - return await this.importExternal(url, targetPath) + throw new Error(`not found ${url}`) } } } From f98da27f221ab9ddf2bbd6c7fa1241c9b3388f04 Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 13:05:04 +0200 Subject: [PATCH 17/20] throw if getPathFromUrl fails --- apps/remix-ide/src/app/files/fileManager.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index 959c1d75a7..e028148a08 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/apps/remix-ide/src/app/files/fileManager.js @@ -173,13 +173,13 @@ class FileManager extends Plugin { try { path = this.getPathFromUrl(path).file } catch (e) { - return console.error(e) + throw e } await this._handleExists(path, `Cannot open file ${path}`) await this._handleIsFile(path, `Cannot open file ${path}`) await this.openFile(path) } catch (e) { - throw new Error(e) + throw e } } @@ -612,7 +612,7 @@ class FileManager extends Plugin { resolved = this.getPathFromUrl(file) file = resolved.file } catch (e) { - return console.error(e) + throw e } const provider = resolved.provider this._deps.config.set('currentFile', file) From cd71a4b19a9c3b675e941e4914eb51d5465869a0 Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 13:19:34 +0200 Subject: [PATCH 18/20] remove uneeded try/catch --- apps/remix-ide/src/app/files/fileManager.js | 26 ++++++--------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index e028148a08..ac9191b1b5 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/apps/remix-ide/src/app/files/fileManager.js @@ -168,19 +168,11 @@ class FileManager extends Plugin { * @returns {void} */ async open (path) { - try { - path = this.limitPluginScope(path) - try { - path = this.getPathFromUrl(path).file - } catch (e) { - throw e - } - await this._handleExists(path, `Cannot open file ${path}`) - await this._handleIsFile(path, `Cannot open file ${path}`) - await this.openFile(path) - } catch (e) { - throw 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) } /** @@ -608,12 +600,8 @@ class FileManager extends Plugin { } else { this.saveCurrentFile() let resolved - try { - resolved = this.getPathFromUrl(file) - file = resolved.file - } catch (e) { - throw e - } + resolved = this.getPathFromUrl(file) + file = resolved.file const provider = resolved.provider this._deps.config.set('currentFile', file) this.openedFiles[file] = file From 04c787d4daa82d03783ef4714595a380df1ff61f Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 25 Aug 2021 13:39:56 +0200 Subject: [PATCH 19/20] linting --- apps/remix-ide/src/app/files/fileManager.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/remix-ide/src/app/files/fileManager.js b/apps/remix-ide/src/app/files/fileManager.js index ac9191b1b5..a378c46cb6 100644 --- a/apps/remix-ide/src/app/files/fileManager.js +++ b/apps/remix-ide/src/app/files/fileManager.js @@ -599,8 +599,7 @@ class FileManager extends Plugin { this.events.emit('noFileSelected') } else { this.saveCurrentFile() - let resolved - resolved = this.getPathFromUrl(file) + const resolved = this.getPathFromUrl(file) file = resolved.file const provider = resolved.provider this._deps.config.set('currentFile', file) From 4dc2e801bfd09c99fafffb6f8bcc9834f261c18b Mon Sep 17 00:00:00 2001 From: Rory Date: Mon, 16 Aug 2021 17:37:55 +1000 Subject: [PATCH 20/20] Fixing broken doco links as mentioned in #1479 --- apps/remix-ide/team-best-practices.md | 2 +- team-best-practices.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/remix-ide/team-best-practices.md b/apps/remix-ide/team-best-practices.md index 770621fc58..3832c3be95 100644 --- a/apps/remix-ide/team-best-practices.md +++ b/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 --- diff --git a/team-best-practices.md b/team-best-practices.md index 56181eef2f..d614f2f702 100644 --- a/team-best-practices.md +++ b/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