diff --git a/lib/apply-fixes.js b/lib/apply-fixes.js new file mode 100644 index 00000000..8f7b1a00 --- /dev/null +++ b/lib/apply-fixes.js @@ -0,0 +1,31 @@ +function applyFixes(fixes, inputSrc) { + if (fixes.length === 0) { + return { + fixed: false + } + } + + let output = '' + + let i = 0 + let fixIndex = 0 + + while (i < inputSrc.length) { + if (fixIndex < fixes.length) { + output += inputSrc.slice(i, fixes[fixIndex].range[0]) + output += fixes[fixIndex].text + i = fixes[fixIndex].range[1] + 1 + fixIndex += 1 + } else { + output += inputSrc.slice(i) + break + } + } + + return { + fixed: true, + output + } +} + +module.exports = applyFixes diff --git a/lib/reporter.js b/lib/reporter.js index c4e7a3e6..92123ff8 100644 --- a/lib/reporter.js +++ b/lib/reporter.js @@ -7,28 +7,35 @@ class Reporter { this.config = config.rules || {} } - addReport(line, column, severity, message, ruleId) { - this.reports.push({ line, column, severity, message, ruleId }) + addReport(line, column, severity, message, ruleId, fix) { + this.reports.push({ line, column, severity, message, ruleId, fix }) } - addMessage(loc, defaultSeverity, message, ruleId) { - this.addMessageExplicitLine(loc.start.line, loc.start.column, defaultSeverity, message, ruleId) + addMessage(loc, defaultSeverity, message, ruleId, fix) { + this.addMessageExplicitLine( + loc.start.line, + loc.start.column, + defaultSeverity, + message, + ruleId, + fix + ) } - addMessageExplicitLine(line, column, defaultSeverity, message, ruleId) { + addMessageExplicitLine(line, column, defaultSeverity, message, ruleId, fix) { const configSeverity = this.severityOf(ruleId) if (this.config[ruleId] !== false && this.commentDirectiveParser.isRuleEnabled(line, ruleId)) { - this.addReport(line, column + 1, configSeverity || defaultSeverity, message, ruleId) + this.addReport(line, column + 1, configSeverity || defaultSeverity, message, ruleId, fix) } } - error(ctx, ruleId, message) { - this.addMessage(ctx.loc, Reporter.SEVERITY.ERROR, message, ruleId) + error(ctx, ruleId, message, fix) { + this.addMessage(ctx.loc, Reporter.SEVERITY.ERROR, message, ruleId, fix) } - warn(ctx, ruleId, message) { - this.addMessage(ctx.loc, Reporter.SEVERITY.WARN, message, ruleId) + warn(ctx, ruleId, message, fix) { + this.addMessage(ctx.loc, Reporter.SEVERITY.WARN, message, ruleId, fix) } errorAt(line, column, ruleId, message) { diff --git a/lib/rule-fixer.js b/lib/rule-fixer.js new file mode 100644 index 00000000..e2240e9c --- /dev/null +++ b/lib/rule-fixer.js @@ -0,0 +1,138 @@ +// taken from eslint (https://github.com/eslint/eslint) + +/** + * @fileoverview An object that creates fix commands for rules. + * @author Nicholas C. Zakas + */ + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +// none! + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Creates a fix command that inserts text at the specified index in the source text. + * @param {int} index The 0-based index at which to insert the new text. + * @param {string} text The text to insert. + * @returns {Object} The fix command. + * @private + */ +function insertTextAt(index, text) { + return { + range: [index, index], + text + } +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +/** + * Creates code fixing commands for rules. + */ + +const ruleFixer = Object.freeze({ + /** + * Creates a fix command that inserts text after the given node or token. + * The fix is not applied until applyFixes() is called. + * @param {ASTNode|Token} nodeOrToken The node or token to insert after. + * @param {string} text The text to insert. + * @returns {Object} The fix command. + */ + insertTextAfter(nodeOrToken, text) { + return this.insertTextAfterRange(nodeOrToken.range, text) + }, + + /** + * Creates a fix command that inserts text after the specified range in the source text. + * The fix is not applied until applyFixes() is called. + * @param {int[]} range The range to replace, first item is start of range, second + * is end of range. + * @param {string} text The text to insert. + * @returns {Object} The fix command. + */ + insertTextAfterRange(range, text) { + return insertTextAt(range[1], text) + }, + + /** + * Creates a fix command that inserts text before the given node or token. + * The fix is not applied until applyFixes() is called. + * @param {ASTNode|Token} nodeOrToken The node or token to insert before. + * @param {string} text The text to insert. + * @returns {Object} The fix command. + */ + insertTextBefore(nodeOrToken, text) { + return this.insertTextBeforeRange(nodeOrToken.range, text) + }, + + /** + * Creates a fix command that inserts text before the specified range in the source text. + * The fix is not applied until applyFixes() is called. + * @param {int[]} range The range to replace, first item is start of range, second + * is end of range. + * @param {string} text The text to insert. + * @returns {Object} The fix command. + */ + insertTextBeforeRange(range, text) { + return insertTextAt(range[0], text) + }, + + /** + * Creates a fix command that replaces text at the node or token. + * The fix is not applied until applyFixes() is called. + * @param {ASTNode|Token} nodeOrToken The node or token to remove. + * @param {string} text The text to insert. + * @returns {Object} The fix command. + */ + replaceText(nodeOrToken, text) { + return this.replaceTextRange(nodeOrToken.range, text) + }, + + /** + * Creates a fix command that replaces text at the specified range in the source text. + * The fix is not applied until applyFixes() is called. + * @param {int[]} range The range to replace, first item is start of range, second + * is end of range. + * @param {string} text The text to insert. + * @returns {Object} The fix command. + */ + replaceTextRange(range, text) { + return { + range, + text + } + }, + + /** + * Creates a fix command that removes the node or token from the source. + * The fix is not applied until applyFixes() is called. + * @param {ASTNode|Token} nodeOrToken The node or token to remove. + * @returns {Object} The fix command. + */ + remove(nodeOrToken) { + return this.removeRange(nodeOrToken.range) + }, + + /** + * Creates a fix command that removes the specified range of text from the source. + * The fix is not applied until applyFixes() is called. + * @param {int[]} range The range to remove, first item is start of range, second + * is end of range. + * @returns {Object} The fix command. + */ + removeRange(range) { + return { + range, + text: '' + } + } +}) + +module.exports = ruleFixer diff --git a/lib/rules/base-checker.js b/lib/rules/base-checker.js index 645507dd..f6ff81c0 100644 --- a/lib/rules/base-checker.js +++ b/lib/rules/base-checker.js @@ -5,16 +5,20 @@ class BaseChecker { this.meta = meta } - error(ctx, message) { - this.reporter.error(ctx, this.ruleId, message) + error(ctx, message, fix) { + this.addReport('error', ctx, message, fix) } - errorAt(line, column, message) { - this.reporter.errorAt(line, column, this.ruleId, message) + errorAt(line, column, message, fix) { + this.error({ loc: { start: { line, column } } }, message, fix) } - warn(ctx, message) { - this.reporter.warn(ctx, this.ruleId, message) + warn(ctx, message, fix) { + this.addReport('warn', ctx, message, fix) + } + + addReport(type, ctx, message, fix) { + this.reporter[type](ctx, this.ruleId, message, this.meta.fixable ? fix : null) } } diff --git a/lib/rules/security/avoid-sha3.js b/lib/rules/security/avoid-sha3.js index e9d963f4..baf8f7f2 100644 --- a/lib/rules/security/avoid-sha3.js +++ b/lib/rules/security/avoid-sha3.js @@ -12,6 +12,7 @@ const meta = { isDefault: false, recommended: true, defaultSetup: 'warn', + fixable: true, schema: [] } @@ -23,7 +24,9 @@ class AvoidSha3Checker extends BaseChecker { Identifier(node) { if (node.name === 'sha3') { - this.error(node, 'Use "keccak256" instead of deprecated "sha3"') + this.error(node, 'Use "keccak256" instead of deprecated "sha3"', fixer => + fixer.replaceTextRange(node.range, 'keccak256') + ) } } } diff --git a/lib/rules/security/avoid-throw.js b/lib/rules/security/avoid-throw.js index 931b01fb..09e339b2 100644 --- a/lib/rules/security/avoid-throw.js +++ b/lib/rules/security/avoid-throw.js @@ -12,6 +12,7 @@ const meta = { isDefault: false, recommended: true, defaultSetup: 'warn', + fixable: true, schema: [] } @@ -22,7 +23,12 @@ class AvoidThrowChecker extends BaseChecker { } ThrowStatement(node) { - this.error(node, '"throw" is deprecated, avoid to use it') + this.error(node, '"throw" is deprecated, avoid to use it', fixer => + // we don't use just `node.range` because ThrowStatement includes the semicolon and the spaces before it + // we know that node.range[0] is the 't' of throw + // we're also pretty sure that 'throw' has 5 letters + fixer.replaceTextRange([node.range[0], node.range[0] + 5], 'revert()') + ) } } diff --git a/solhint.js b/solhint.js index 0b578d07..cdc4602c 100755 --- a/solhint.js +++ b/solhint.js @@ -8,6 +8,8 @@ const process = require('process') const linter = require('./lib/index') const { applyExtends, loadConfig } = require('./lib/config/config-file') const { validate } = require('./lib/config/config-validator') +const applyFixes = require('./lib/apply-fixes') +const ruleFixer = require('./lib/rule-fixer') const packageJson = require('./package.json') function init() { @@ -21,6 +23,7 @@ function init() { .option('-c, --config [file_name]', 'file to use as your .solhint.json') .option('-q, --quiet', 'report errors only - default: false') .option('--ignore-path [file_name]', 'file to use as your .solhintignore') + .option('--fix', 'automatically fix problems') .description('Linter for Solidity programming language') .action(execMainAction) @@ -58,6 +61,24 @@ function execMainAction() { const warningsCount = reports.reduce((acc, i) => acc + i.warningCount, 0) const warningsNumberExceeded = program.maxWarnings >= 0 && warningsCount > program.maxWarnings + if (program.fix) { + for (const report of reports) { + const inputSrc = fs.readFileSync(report.filePath).toString() + + const fixes = _(report.reports) + .filter(x => x.fix) + .map(x => x.fix(ruleFixer)) + .sort((a, b) => a.range[0] - b.range[0]) + .value() + + const { fixed, output } = applyFixes(fixes, inputSrc) + if (fixed) { + report.reports = report.reports.filter(x => !x.fix) + fs.writeFileSync(report.filePath, output) + } + } + } + if (program.quiet) { // filter the list of reports, to set errors only. reports[0].reports = reports[0].reports.filter(i => i.severity === 2) diff --git a/test/common/apply-fixes.js b/test/common/apply-fixes.js new file mode 100644 index 00000000..0a88665d --- /dev/null +++ b/test/common/apply-fixes.js @@ -0,0 +1,78 @@ +const assert = require('assert') + +const applyFixes = require('../../lib/apply-fixes') + +describe('applyFixes', () => { + it('should work when there are no reports', () => { + const inputSrc = 'contract Foo {}' + const fixes = [] + + const { fixed } = applyFixes(fixes, inputSrc) + + assert.equal(fixed, false) + }) + + it('should work for a single replace', () => { + const inputSrc = ` +contract Foo { + function foo() { + throw; + } +}`.trim() + + const fixes = [ + { + range: [38, 42], + text: 'revert()' + } + ] + + const { fixed, output } = applyFixes(fixes, inputSrc) + + assert.equal(fixed, true) + assert.equal( + output, + ` +contract Foo { + function foo() { + revert(); + } +}`.trim() + ) + }) + + it('should work for two fixes', () => { + const inputSrc = ` +contract Foo { + function foo() { + throw; + throw; + } +}`.trim() + + const fixes = [ + { + range: [38, 42], + text: 'revert()' + }, + { + range: [49, 53], + text: 'revert()' + } + ] + + const { fixed, output } = applyFixes(fixes, inputSrc) + + assert.equal(fixed, true) + assert.equal( + output, + ` +contract Foo { + function foo() { + revert(); + revert(); + } +}`.trim() + ) + }) +}) diff --git a/test/rules/security/mark-callable-contracts.js b/test/rules/security/mark-callable-contracts.js index fd99809c..64dc5540 100644 --- a/test/rules/security/mark-callable-contracts.js +++ b/test/rules/security/mark-callable-contracts.js @@ -117,6 +117,21 @@ describe('Linter - mark-callable-contracts', () => { rules: { 'mark-callable-contracts': 'warn' } }) + assert.equal(report.warningCount, 0) + }) + it('should not return error for an event defined after function', () => { + const code = contractWith(` + function b() public { + emit UpdatedToken(); + } + + event UpdatedToken(); + `) + + const report = linter.processStr(code, { + rules: { 'mark-callable-contracts': 'warn' } + }) + assert.equal(report.warningCount, 0) }) })