diff --git a/.travis.yml b/.travis.yml index e7ba38a7..1e9e449e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,11 @@ language: node_js node_js: + - "10" - "stable" +before_script: npm i -g npm@6.9 + script: - npm run lint - npm test diff --git a/lib/comment-directive-parser.js b/lib/comment-directive-parser.js index 34a743a2..316f89ce 100644 --- a/lib/comment-directive-parser.js +++ b/lib/comment-directive-parser.js @@ -1,21 +1,22 @@ class CommentDirectiveParser { constructor(tokens) { - this.lastLine = tokens.tokenSource.line + const lastToken = tokens[tokens.length - 1] + this.lastLine = lastToken ? lastToken.loc.end.line : 0 this.ruleStore = new RuleStore(this.lastLine) this.parseComments(tokens) } parseComments(tokens) { - const items = tokens.filterForChannel(0, tokens.tokens.length - 1, 1) - if (items) { - items.forEach(this.onComment.bind(this)) - } + const items = tokens.filter( + token => token.type === 'Keyword' && /^(\/\/|\/\*)/.test(token.value) + ) + items.forEach(item => this.onComment(item)) } onComment(lexema) { - const text = lexema.text - const curLine = lexema.line + const text = lexema.value + const curLine = lexema.loc.start.line const ruleStore = this.ruleStore if (text.includes('solhint-disable-line')) { diff --git a/lib/common/ast-types.js b/lib/common/ast-types.js index 6647c198..a978a8db 100644 --- a/lib/common/ast-types.js +++ b/lib/common/ast-types.js @@ -1,11 +1,11 @@ -const { typeOf } = require('./tree-traversing') +const fallbackVisibility = ['external', 'public'] -function isFallbackFunction(ctx) { - return ctx.children && ctx.children[1] && ctx.children[1].constructor.name !== 'IdentifierContext' +function isFallbackFunction(node) { + return node.name === '' && fallbackVisibility.includes(node.visibility) } -function isFunctionDefinition(ctx) { - return typeOf(ctx) === 'functionDefinition' +function isFunctionDefinition(node) { + return node.type === 'FunctionDefinition' } module.exports = { diff --git a/lib/common/tree-traversing.js b/lib/common/tree-traversing.js index 2c315b41..a56e3d48 100644 --- a/lib/common/tree-traversing.js +++ b/lib/common/tree-traversing.js @@ -1,6 +1,6 @@ class TreeTraversing { - statementNotContains(ctx, type) { - const statement = this.findParentStatement(ctx) + statementNotContains(node, type) { + const statement = this.findParentStatement(node) if (!statement) { return false @@ -11,29 +11,25 @@ class TreeTraversing { return itemOfType !== null } - findParentStatement(ctx) { - while (ctx.parentCtx != null && ctx.parentCtx.constructor.name !== 'StatementContext') { - ctx = ctx.parentCtx + findParentStatement(node) { + while (node.parent != null && !node.parent.type.includes('Statement')) { + node = node.parent } - return ctx.parentCtx + return node.parent } - findParentType(ctx, type) { - while (ctx.parentCtx != null && ctx.parentCtx.constructor.name !== type) { - ctx = ctx.parentCtx + findParentType(node, type) { + while (node.parent !== undefined && node.parent.type !== type) { + node = node.parent } - return ctx.parentCtx + return node.parent || null } - findDownType(ctx, type) { - if (!ctx || ctx.constructor.name === type) { - return ctx - } else if (ctx.children) { - const items = ctx.children.map(i => this.findDownType(i, type)).filter(i => i !== null) - - return (items.length > 0 && items[0]) || null + findDownType(node, type) { + if (!node || node.type === type) { + return node } else { return null } @@ -72,20 +68,20 @@ TreeTraversing.typeOf = function typeOf(ctx) { return typeName[0].toLowerCase() + typeName.substring(1) } -TreeTraversing.hasMethodCalls = function hasMethodCalls(ctx, methodNames) { - const text = ctx.getText() +TreeTraversing.hasMethodCalls = function hasMethodCalls(node, methodNames) { + const text = node.memberName - return methodNames.some(i => text.includes(`${i}(`)) + return methodNames.includes(text) } -TreeTraversing.findPropertyInParents = function findPropertyInParents(ctx, property) { - let curCtx = ctx +TreeTraversing.findPropertyInParents = function findPropertyInParents(node, property) { + let curNode = node - while (curCtx !== null && !curCtx[property]) { - curCtx = curCtx.parentCtx + while (curNode !== undefined && !curNode[property]) { + curNode = curNode.parent } - return curCtx && curCtx[property] + return curNode && curNode[property] } module.exports = TreeTraversing diff --git a/lib/index.js b/lib/index.js index ff966088..5899f64a 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,25 +1,36 @@ const fs = require('fs') -const antlr4 = require('antlr4') +const parser = require('solidity-parser-antlr') const glob = require('glob') const ignore = require('ignore') -const SolidityLexer = require('./grammar/SolidityLexer').SolidityLexer -const SolidityParser = require('./grammar/SolidityParser').SolidityParser +const astParents = require('ast-parents') const Reporter = require('./reporter') const TreeListener = require('./tree-listener') const checkers = require('./rules/index') -function processStr(inputStr, config = {}, fileName = '') { - const chars = new antlr4.InputStream(inputStr) - const lexer = new SolidityLexer(chars) - const tokens = new antlr4.CommonTokenStream(lexer) - const parser = new SolidityParser(tokens) - parser.buildParseTrees = true +function parseInput(inputStr) { + try { + // first we try to parse the string as we normally do + return parser.parse(inputStr, { loc: true, range: true }) + } catch (e) { + try { + // using 'loc' may throw when inputStr is empty or only has comments + return parser.parse(inputStr, {}) + } catch (error) { + // if the parser fails in both scenarios (with and without 'loc') + // we throw the error, as we may be facing an invalid Solidity file + throw new Error(error) + } + } +} - const tree = parser.sourceUnit() +function processStr(inputStr, config = {}, fileName = '') { + const ast = parseInput(inputStr) + const tokens = parser.tokenize(inputStr, { loc: true }) const reporter = new Reporter(tokens, config) + const listener = new TreeListener(checkers(reporter, config, inputStr, tokens, fileName)) - const listener = new TreeListener(checkers(reporter, config, inputStr, fileName)) - antlr4.tree.ParseTreeWalker.DEFAULT.walk(listener, tree) + astParents(ast) + parser.visit(ast, listener) return reporter } diff --git a/lib/reporter.js b/lib/reporter.js index 04cfcaf7..c4e7a3e6 100644 --- a/lib/reporter.js +++ b/lib/reporter.js @@ -1,10 +1,9 @@ const CommentDirectiveParser = require('./comment-directive-parser') class Reporter { - constructor(tokenStream, config) { - this.commentDirectiveParser = new CommentDirectiveParser(tokenStream) + constructor(tokens, config) { + this.commentDirectiveParser = new CommentDirectiveParser(tokens) this.reports = [] - this.tokenStream = tokenStream this.config = config.rules || {} } @@ -12,11 +11,8 @@ class Reporter { this.reports.push({ line, column, severity, message, ruleId }) } - addMessage(interval, defaultSeverity, message, ruleId) { - const line = this.tokenStream.get(interval.start).line - const charAtLine = this.tokenStream.get(interval.start).column - - this.addMessageExplicitLine(line, charAtLine, defaultSeverity, message, ruleId) + addMessage(loc, defaultSeverity, message, ruleId) { + this.addMessageExplicitLine(loc.start.line, loc.start.column, defaultSeverity, message, ruleId) } addMessageExplicitLine(line, column, defaultSeverity, message, ruleId) { @@ -28,11 +24,11 @@ class Reporter { } error(ctx, ruleId, message) { - this.addMessage(ctx.getSourceInterval(), Reporter.SEVERITY.ERROR, message, ruleId) + this.addMessage(ctx.loc, Reporter.SEVERITY.ERROR, message, ruleId) } warn(ctx, ruleId, message) { - this.addMessage(ctx.getSourceInterval(), Reporter.SEVERITY.WARN, message, ruleId) + this.addMessage(ctx.loc, Reporter.SEVERITY.WARN, message, ruleId) } errorAt(line, column, ruleId, message) { diff --git a/lib/rules/align/array-declaration-spaces.js b/lib/rules/align/array-declaration-spaces.js deleted file mode 100644 index 14b7c364..00000000 --- a/lib/rules/align/array-declaration-spaces.js +++ /dev/null @@ -1,64 +0,0 @@ -const BaseChecker = require('./../base-checker') -const { hasNoSpacesBefore } = require('./../../common/tokens') - -const ruleId = 'array-declaration-spaces' -const meta = { - type: 'align', - - docs: { - description: 'Array declaration must not contains spaces.', - category: 'Style Guide Rules', - examples: { - good: [ - { - description: 'Array declaration without spaces', - code: require('../../../test/fixtures/align/array_declaration') - } - ], - bad: [ - { - description: 'Array declaration with spaces', - code: require('../../../test/fixtures/align/array_declaration_with_spaces') - } - ] - } - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - schema: [] -} - -class ArrayDeclarationSpacesChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - enterTypeName(ctx) { - this.validateSpaceBeforeBracket(ctx, '[') - this.validateSpaceBeforeBracket(ctx, ']') - } - - validateSpaceBeforeBracket(ctx, bracketSymbol) { - const bracket = this._bracket(ctx, bracketSymbol) - - if (bracket && !hasNoSpacesBefore(bracket)) { - this.makeReport(bracket) - } - } - - _bracket(ctx, bracketSymbol) { - const children = ctx.children - const bracket = children && children.filter(i => i.getText() === bracketSymbol) - - return bracket.length === 1 ? bracket[0] : null - } - - makeReport(ctx) { - this.error(ctx, 'Array declaration must not contains spaces') - } -} - -module.exports = ArrayDeclarationSpacesChecker diff --git a/lib/rules/align/bracket-align.js b/lib/rules/align/bracket-align.js deleted file mode 100644 index 00ee05a8..00000000 --- a/lib/rules/align/bracket-align.js +++ /dev/null @@ -1,131 +0,0 @@ -const _ = require('lodash') -const BaseChecker = require('./../base-checker') -const { hasSpaceBefore, onSameLine, prevToken, startOf } = require('./../../common/tokens') -const { isFunctionDefinition } = require('./../../common/ast-types') - -const ruleId = 'bracket-align' -const meta = { - type: 'align', - - docs: { - description: - 'Open bracket must be on same line. It must be indented by other constructions by space.', - category: 'Style Guide Rules', - examples: { - good: [ - { - description: 'Correctly aligned function brackets', - code: require('../../../test/fixtures/align/correctly_aligned_function_brackets') - } - ], - bad: [ - { - description: 'Incorrectly aligned function brackets', - code: require('../../../test/fixtures/align/incorrectly_aligned_function_brackets') - } - ] - } - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - schema: [] -} - -class BracketAlign extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - enterBlock(ctx) { - this.validateBlock(ctx) - } - - enterContractDefinition(ctx) { - this.validateBlock(ctx) - } - - enterStructDefinition(ctx) { - this.validateBlock(ctx) - } - - enterEnumDefinition(ctx) { - this.validateBlock(ctx) - } - - enterImportDirective(ctx) { - this.validateBlock(ctx) - } - - validateBlock(ctx) { - const bracket = new Block(ctx).openBracket() - - if (bracket && !bracket.isCorrectAligned()) { - this._error(bracket) - } - } - - _error({ ctx, errorMessage }) { - this.error(ctx, errorMessage) - } -} - -class Block { - constructor(ctx) { - this.ctx = ctx - } - - openBracket() { - const bracketCtx = this._openBracketCtx() - - return bracketCtx ? this._makeBracketObj(bracketCtx) : null - } - - _openBracketCtx() { - const children = this.ctx.children - const openBrackets = children && children.filter(i => i.getText() === '{') - - return _.size(openBrackets) === 1 ? openBrackets[0] : null - } - - _makeBracketObj(ctx) { - return isFunctionDefinition(this.ctx.parentCtx) - ? new FunctionOpenBracket(ctx) - : new UsualOpenBracket(ctx) - } -} - -class OpenBracket { - constructor(ctx) { - this.ctx = ctx - this.errorMessage = 'Open bracket must be indented by other constructions by space' - } - - isCorrectAligned() { - return hasSpaceBefore(this.ctx) - } -} - -class FunctionOpenBracket extends OpenBracket {} - -class UsualOpenBracket extends OpenBracket { - constructor(ctx) { - super(ctx) - this.errorMessage = - 'Open bracket must be on same line. It must be indented by other constructions by space' - } - - isOnSameLineWithPreviousToken() { - const ctx = this.ctx - - return onSameLine(startOf(ctx), prevToken(ctx)) - } - - isCorrectAligned() { - return super.isCorrectAligned() && this.isOnSameLineWithPreviousToken() - } -} - -module.exports = BracketAlign diff --git a/lib/rules/align/expression-indent.js b/lib/rules/align/expression-indent.js deleted file mode 100644 index b6266c82..00000000 --- a/lib/rules/align/expression-indent.js +++ /dev/null @@ -1,110 +0,0 @@ -const BaseChecker = require('./../base-checker') -const { - StatementsIndentValidator, - Term, - Rule -} = require('./../../common/statements-indent-validator') - -const ruleId = 'expression-indent' -const meta = { - type: 'align', - - docs: { - description: 'Expression indentation is incorrect.', - category: 'Style Guide Rules', - examples: { - good: [ - { - description: 'Expressions with correct indentation', - code: require('../../../test/fixtures/align/expressions_with_correct_indents').join('\n') - } - ], - bad: [ - { - description: 'Expressions with incorrect indentation', - code: require('../../../test/fixtures/align/expressions_with_incorrect_indents').join( - '\n' - ) - } - ] - } - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - schema: [] -} - -class ExpressionIndentChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - enterExpression(ctx) { - const expression = Rule.expression - const term = Term.term - - new StatementsIndentValidator(ctx) - .cases( - term('new') - .space() - .rule('typeName'), - expression() - .noSpacesAround('[') - .expression() - .noSpaces() - .term(']'), - expression() - .noSpacesAround('(') - .rule('functionCallArguments') - .noSpaces() - .term(')'), - expression() - .noSpacesAround('.') - .identifier(), - term('(') - .noSpaces() - .expression() - .noSpaces() - .term(')'), - expression() - .spaceAround('=', '|=', '^=', '&=', '<<=') - .expression(), - expression() - .spaceAround('>>=', '+=', '-=', '*=', '/=', '%=') - .expression(), - expression() - .spaceAround('==', '!=', '<', '>', '<=', '>=') - .expression(), - expression() - .spaceAroundOrNot('^', '**', '*', '/', '%', '+', '-') - .expression(), - expression() - .spaceAroundOrNot('<<', '>>', '&', '|', '&&', '||') - .expression(), - expression() - .spaceAround('?') - .expression() - .spaceAround(':') - .expression(), - term('!', '~', '+', '-', '++', '--') - .noSpaces() - .expression(), - term('after', 'delete') - .space() - .expression(), - expression() - .noSpaces() - .term('++', '--') - ) - .errorsTo(this._error.bind(this)) - } - - _error(ctx, message) { - this.error(ctx, `Expression indentation is incorrect. ${message}`) - } -} - -module.exports = ExpressionIndentChecker diff --git a/lib/rules/align/indent.js b/lib/rules/align/indent.js deleted file mode 100644 index a3c137cb..00000000 --- a/lib/rules/align/indent.js +++ /dev/null @@ -1,425 +0,0 @@ -const _ = require('lodash') -const { typeOf } = require('./../../common/tree-traversing') -const { columnOf, lineOf, stopLine } = require('./../../common/tokens') -const { severityDescription } = require('../../doc/utils') - -const ruleId = 'indent' -const DEFAULT_SEVERITY = 'error' -const DEFAULT_INDENTS = 4 -const meta = { - type: 'align', - - docs: { - description: 'Indentation is incorrect.', - category: 'Style Guide Rules', - options: [ - { - description: severityDescription, - default: DEFAULT_SEVERITY - }, - { - description: 'Number of indents', - default: DEFAULT_INDENTS - } - ], - examples: { - good: [ - { - description: 'Contract with correct indentation', - code: require('../../../test/fixtures/align/correctly_indented_contract') - } - ], - bad: [ - { - description: 'Contract with incorrect indentation', - code: require('../../../test/fixtures/align/incorrectly_indented_contract') - } - ] - } - }, - - isDefault: true, - recommended: true, - defaultSetup: [DEFAULT_SEVERITY, DEFAULT_INDENTS], - - schema: [ - { - type: 'array', - items: [{ type: 'integer' }], - uniqueItems: true, - minItems: 2 - } - ] -} - -class IndentChecker { - constructor(reporter, config) { - this.reporter = reporter - this.ruleId = ruleId - this.meta = meta - this.linesWithError = [] - - const indent = this.parseConfig(config).indent || 4 - const indentUnit = this.parseConfig(config).unit || 'spaces' - - this.blockValidator = new BlockValidator(indent, indentUnit, reporter, this.ruleId) - this.nestedSingleLineValidator = new NestedSingleLineValidator( - indent, - indentUnit, - reporter, - this.ruleId - ) - this.baseIndentMultiplicityValidator = new BaseIndentMultiplicityValidator( - indent, - reporter, - this.ruleId - ) - } - - enterBlock(ctx) { - this.blockValidator.validateBlock(ctx) - } - - enterContractDefinition(ctx) { - this.blockValidator.validateBlock(ctx) - } - - enterStructDefinition(ctx) { - this.blockValidator.validateBlock(ctx) - } - - enterEnumDefinition(ctx) { - this.blockValidator.validateBlock(ctx) - } - - enterImportDirective(ctx) { - this.blockValidator.validateBlock(ctx) - } - - enterFunctionCallArguments(ctx) { - this.blockValidator.validateBlock(ctx) - } - - enterIfStatement(ctx) { - const THEN_STATEMENT_POSITION = 4 - const ELSE_STATEMENT_POSITION = 6 - const STATEMENTS_POSITION = [THEN_STATEMENT_POSITION, ELSE_STATEMENT_POSITION] - - this.nestedSingleLineValidator.validateMultiple(ctx, STATEMENTS_POSITION) - } - - enterWhileStatement(ctx) { - const STATEMENT_POSITION = 4 - - this.nestedSingleLineValidator.validate(ctx, STATEMENT_POSITION) - } - - enterDoWhileStatement(ctx) { - this.nestedSingleLineValidator.validate(ctx, 1) - } - - enterForStatement(ctx) { - this.nestedSingleLineValidator.validate(ctx, ctx.children.length - 1) - } - - enterSourceUnit(ctx) { - ctx.children - .filter(i => i.getText() !== '') - .forEach(curNode => - this.blockValidator.validateNode(0)(curNode, lineOf(curNode), columnOf(curNode)) - ) - } - - exitSourceUnit(ctx) { - const linesWithErrors = this.getLinesWithError() - - this.baseIndentMultiplicityValidator.validate(linesWithErrors, ctx) - } - - parseConfig(config) { - const rules = (config && config.rules) || {} - if (!(rules && rules.indent && rules.indent.length === 2)) { - return {} - } - - const indentConf = rules.indent[1] - if (indentConf === 'tabs') { - return { indent: 1, unit: 'tabs' } - } else if (_.isNumber(indentConf)) { - return { indent: indentConf, unit: 'spaces' } - } else { - return {} - } - } - - getLinesWithError() { - return [].concat( - this.nestedSingleLineValidator.linesWithError, - this.blockValidator.linesWithError - ) - } -} - -class Block { - constructor(ctx) { - this.ctx = ctx - this.startBracketIndex = _.memoize(this._startBracketIndex.bind(this)) - this.endBracketIndex = _.memoize(this._endBracketIndex.bind(this)) - } - - _startBracketIndex() { - const children = this.ctx.children - return children && children.map(i => i.getText()).indexOf('{') - } - - hasStartBracket() { - return this.startBracketIndex() !== null && this.startBracketIndex() >= 0 - } - - startBracket() { - return this.ctx.children[this.startBracketIndex()] - } - - startBracketLine() { - return this.startBracket().symbol.line - } - - _endBracketIndex() { - return this.ctx.children.map(i => i.getText()).indexOf('}') - } - - endBracket() { - const children = this.ctx.children - return children[children.length - 1] - } - - endBracketLine() { - return this.endBracket().symbol.line - } - - endBracketColumn() { - return this.endBracket().symbol.column - } - - isBracketsOnSameLine() { - return this.startBracketLine() === this.endBracketLine() - } - - forEachNestedNode(callback) { - for (let i = this.startBracketIndex() + 1; i < this.endBracketIndex(); i += 1) { - const curItem = this.ctx.children[i] - const isTerm = curItem.symbol - - if (!isTerm && callback) { - callback(curItem, lineOf(curItem), columnOf(curItem)) - } - } - } -} - -class KnowLineValidator { - constructor(indent, indentUnit, reporter, ruleId) { - this.indent = indent - this.indentUnit = indentUnit - this.reporter = reporter - this.linesWithError = [] - this.ruleId = ruleId - } - - makeReportCorrectLine(line, col, correctIndent) { - this.linesWithError.push(line) - - const message = `Expected indentation of ${correctIndent} ${this.indentUnit} but found ${col}` - this.reporter.errorAt(line, col, this.ruleId, message) - } -} - -class BlockValidator extends KnowLineValidator { - validateBlock(ctx) { - const block = new Block(ctx) - - if (!block.hasStartBracket() || block.isBracketsOnSameLine()) { - return - } - - this.validateIndentOfNestedElements(block) - - this.validateEndBracketIndent(block) - } - - validateIndentOfNestedElements(block) { - const requiredIndent = correctIndentOf(alreadyVerifiedRootNode(block.ctx)) + this.indent - - block.forEachNestedNode(this.validateNode(requiredIndent)) - } - - validateNode(requiredIndent) { - return (curItem, curLine, curColumn) => { - if (curColumn !== requiredIndent) { - this.makeReportCorrectLine(curLine, curColumn, requiredIndent) - curItem.indentError = { indent: curColumn, correctIndent: requiredIndent } - } - } - } - - validateEndBracketIndent(block) { - const endBracketCorrectIndent = correctIndentOf(alreadyVerifiedRootNode(block.ctx)) - - if (endBracketCorrectIndent !== block.endBracketColumn()) { - this.makeReportCorrectLine( - block.endBracketLine(), - block.endBracketColumn(), - endBracketCorrectIndent - ) - } - } -} - -class NestedSingleLineValidator extends KnowLineValidator { - validateMultiple(ctx, indexes) { - indexes.forEach(index => this.validate(ctx, index)) - } - - validate(ctx, index) { - if (ctx.children.length <= index) { - return - } - - const statement = ctx.children[index] - const statementColumn = columnOf(statement) - const statementLine = lineOf(statement) - const start = ctx.start - const requiredIndent = correctIndentOf(ctx.parentCtx) + this.indent - - if ( - !['BlockContext', 'IfStatementContext'].includes(statement.children[0].constructor.name) && - statementColumn !== requiredIndent && - statementLine !== start.line - ) { - this.makeReportCorrectLine(statementLine, statementColumn, requiredIndent) - statement.indentError = { - indent: statementColumn, - correctIndent: correctIndentOf(ctx.parentCtx) + this.indent - } - } - } -} - -class BaseIndentMultiplicityValidator { - constructor(indent, reporter, ruleId) { - this.reporter = reporter - this.indent = indent - this.firstIndent = new Map() - this.ruleId = ruleId - } - - validate(linesWithError, ctx) { - const tokens = ctx.parser._input.tokens.filter(i => i.channel === 0 && i.type >= 0) - - tokens.forEach(this.applyTokenIndent.bind(this)) - - for (const curLineStr in this.firstIndent) { - const curLine = Number(curLineStr) - if (linesWithError.includes(Number(curLine))) { - continue - } - - const curIndent = this.firstIndent[curLine] - if (this.isNotValidForBaseIndent(curIndent)) { - this.error(curLine, curIndent) - } - } - } - - applyTokenIndent(token) { - const line = token.line - const column = token.column - const curIndent = this.firstIndent[line] - - if (curIndent > column || _.isUndefined(curIndent)) { - this.firstIndent[line] = column - } - } - - isNotValidForBaseIndent(indent) { - return indent % this.indent !== 0 - } - - error(line, col) { - this.reporter.errorAt(line, col, this.ruleId, 'Indentation is incorrect') - } -} - -function correctIndentOf(ctx) { - let curIndent = columnOf(ctx) - let curCtx = ctx - - do { - if (curCtx.indentError) { - curIndent = correctIndent(curIndent, curCtx.indentError) - return curIndent - } - - curCtx = curCtx.parentCtx - } while (curCtx !== null && lineOf(ctx) === lineOf(curCtx)) - - return curIndent -} - -function correctIndent(curIndent, indentError) { - return curIndent - indentError.indent + indentError.correctIndent -} - -function firstNodeOfLine(ctx) { - let rootCtx = ctx - - while ( - rootCtx.parentCtx && - rootCtx.start.line === rootCtx.parentCtx.start.line && - !['SourceUnitContext'].includes(rootCtx.parentCtx.constructor.name) - ) { - rootCtx = rootCtx.parentCtx - } - - let resultNode = rootCtx - - if (rootCtx.parentCtx !== null) { - const curParent = rootCtx.parentCtx - const rootIdx = curParent.children.indexOf(rootCtx) - - for (let i = rootIdx - 1; i >= 0; i -= 1) { - const curChild = curParent.children[i] - - if (stopLine(curChild) === lineOf(rootCtx)) { - resultNode = curChild - } - } - } - - if (lineOf(ctx) !== lineOf(resultNode)) { - resultNode = firstNodeOfLine(resultNode) - } - - return resultNode -} - -function alreadyVerifiedRootNode(ctx) { - const rootNode = firstNodeOfLine(ctx) - - // in case when open bracket is the first on the line - if (rootNode === ctx && typeOf(ctx) === 'block' && firstChildText(ctx) === '{') { - const parent = ctx.parentCtx - - if (typeOf(parent) === 'functionDefinition' || typeOf(parent) === 'constructorDefinition') { - return firstNodeOfLine(parent) - } - } - - return rootNode -} - -function firstChildText(ctx) { - return ctx.children && ctx.children[0] && ctx.children[0].getText() -} - -module.exports = IndentChecker diff --git a/lib/rules/align/index.js b/lib/rules/align/index.js deleted file mode 100644 index bc5e7362..00000000 --- a/lib/rules/align/index.js +++ /dev/null @@ -1,21 +0,0 @@ -const ArrayDeclarationSpacesChecker = require('./array-declaration-spaces') -const BracketAlign = require('./bracket-align') -const ExpressionIndentChecker = require('./expression-indent') -const IndentChecker = require('./indent') -const NoMixTabsAndSpacesChecker = require('./no-mix-tabs-and-spaces') -const NoSpacesBeforeSemicolonChecker = require('./no-spaces-before-semicolon') -const SpaceAfterCommaChecker = require('./space-after-comma') -const StatementsAlignChecker = require('./statement-indent') - -module.exports = function checkers(reporter, config) { - return [ - new ArrayDeclarationSpacesChecker(reporter), - new BracketAlign(reporter), - new ExpressionIndentChecker(reporter), - new IndentChecker(reporter, config), - new NoMixTabsAndSpacesChecker(reporter, config), - new NoSpacesBeforeSemicolonChecker(reporter), - new SpaceAfterCommaChecker(reporter), - new StatementsAlignChecker(reporter) - ] -} diff --git a/lib/rules/align/no-mix-tabs-and-spaces.js b/lib/rules/align/no-mix-tabs-and-spaces.js deleted file mode 100644 index 93510e17..00000000 --- a/lib/rules/align/no-mix-tabs-and-spaces.js +++ /dev/null @@ -1,55 +0,0 @@ -const BaseChecker = require('./../base-checker') - -const ruleId = 'no-mix-tabs-and-spaces' -const meta = { - type: 'align', - - docs: { - description: 'Mixed tabs and spaces.', - category: 'Style Guide Rules', - examples: { - bad: [ - { - description: 'Expression with mixed tabs and spaces', - code: require('../../../test/fixtures/align/expression_with_mixed_tabs_and_spaces') - } - ] - } - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - schema: [] -} - -class NoMixTabsAndSpacesChecker extends BaseChecker { - constructor(reporter, config) { - super(reporter, ruleId, meta) - - const configDefined = config && config.rules && config.rules.indent && config.rules.indent[1] - this.spacer = (configDefined && config.rules.indent[1] === 'tabs' && 'tabs') || 'spaces' - } - - enterSourceUnit(ctx) { - ctx.parser._input.tokenSource._input.strdata - .split('\n') - .map((i, index) => [i.replace(/[^\s]+.*/, ''), index]) - .filter(args => this.isMixTabsAndSpaces(args[0])) - .forEach(args => this._error(args[1])) - } - - isMixTabsAndSpaces(prefixLine) { - const disallowedSpacer = this.spacer === 'tabs' ? ' ' : '\t' - - return prefixLine.includes(disallowedSpacer) - } - - _error(line) { - const message = `Mixed tabs and spaces. Allowed only ${this.spacer}` - this.errorAt(line + 1, 0, message) - } -} - -module.exports = NoMixTabsAndSpacesChecker diff --git a/lib/rules/align/no-spaces-before-semicolon.js b/lib/rules/align/no-spaces-before-semicolon.js deleted file mode 100644 index 296f59e7..00000000 --- a/lib/rules/align/no-spaces-before-semicolon.js +++ /dev/null @@ -1,79 +0,0 @@ -const BaseChecker = require('./../base-checker') -const { - noSpaces, - prevTokenFromToken, - BaseTokenList, - Token, - AlignValidatable -} = require('./../../common/tokens') - -const ruleId = 'no-spaces-before-semicolon' -const meta = { - type: 'align', - - docs: { - description: 'Semicolon must not have spaces before.', - category: 'Style Guide Rules', - examples: { - good: [ - { - description: 'Expression with correct semicolon align', - code: require('../../../test/fixtures/align/expressions_with_correct_semicolon_align').join( - '\n' - ) - } - ], - bad: [ - { - description: 'Expression with incorrect semicolon align', - code: require('../../../test/fixtures/align/expressions_with_incorrect_semicolon_align').join( - '\n' - ) - } - ] - } - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - schema: [] -} - -class NoSpacesBeforeSemicolonChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - exitSourceUnit(ctx) { - TokenList.from(ctx) - .semicolonTokens() - .filter(curToken => curToken.isIncorrectAligned()) - .forEach(this._error.bind(this)) - } - - _error(curToken) { - const message = 'Semicolon must not have spaces before' - this.errorAt(curToken.line, curToken.column, message) - } -} - -class TokenList extends BaseTokenList { - semicolonTokens() { - return this.tokens - .filter(i => i.text === ';') - .map(curToken => new SemicolonToken(this.tokens, curToken)) - } -} - -class SemicolonToken extends AlignValidatable(Token) { - isCorrectAligned() { - const curToken = this.curToken - const prevToken = prevTokenFromToken(this.tokens, curToken) - - return noSpaces(curToken, prevToken) - } -} - -module.exports = NoSpacesBeforeSemicolonChecker diff --git a/lib/rules/align/space-after-comma.js b/lib/rules/align/space-after-comma.js deleted file mode 100644 index 8b42f7b9..00000000 --- a/lib/rules/align/space-after-comma.js +++ /dev/null @@ -1,106 +0,0 @@ -const BaseChecker = require('./../base-checker') -const { - hasSpace, - nextTokenFromToken, - BaseTokenList, - Token, - AlignValidatable -} = require('./../../common/tokens') -const { noSpaces, prevTokenFromToken } = require('./../../common/tokens') - -const ruleId = 'space-after-comma' -const meta = { - type: 'align', - - docs: { - description: 'Comma must be separated from next element by space.', - category: 'Style Guide Rules', - examples: { - good: [ - { - description: 'Expression with correct comma align', - code: require('../../../test/fixtures/align/expressions_with_correct_comma_align')[0] - } - ], - bad: [ - { - description: 'Expression with incorrect comma align', - code: require('../../../test/fixtures/align/expressions_with_incorrect_comma_align')[0] - } - ] - } - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - schema: [] -} - -class SpaceAfterCommaChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - exitSourceUnit(ctx) { - TokenList.from(ctx) - .commaTokens() - .filter(curToken => curToken.isIncorrectAligned()) - .forEach(this._error.bind(this)) - } - - _error(curToken) { - const message = 'Comma must be separated from next element by space' - this.errorAt(curToken.line, curToken.column, message) - } -} - -class TokenList extends BaseTokenList { - commaTokens() { - return this.tokens - .filter(i => i.text === ',') - .map(curToken => new CommaToken(this.tokens, curToken)) - } -} - -class CommaToken extends AlignValidatable(Token) { - isCorrectAligned() { - return ( - (this._isNoSpacesBefore() || this._isCommaAfterComma()) && - (this._isSpaceAfter() || this._isCommaInEndOfExpression()) - ) - } - - _isNoSpacesBefore() { - return noSpaces(this.curToken, this._prevToken()) - } - - _isSpaceAfter() { - return hasSpace(this._nextToken(), this.curToken) - } - - _isCommaInEndOfExpression() { - const curToken = this.curToken - const _nextToken = nextTokenFromToken(this.tokens, curToken) - - return [')', '}'].includes(_nextToken.text) - } - - _isCommaAfterComma() { - const curToken = this.curToken - const _prevToken = prevTokenFromToken(this.tokens, curToken) - - return [','].includes(_prevToken.text) - } - - _prevToken() { - return prevTokenFromToken(this.tokens, this.curToken) - } - - _nextToken() { - return nextTokenFromToken(this.tokens, this.curToken) - } -} - -module.exports = SpaceAfterCommaChecker diff --git a/lib/rules/align/statement-indent.js b/lib/rules/align/statement-indent.js deleted file mode 100644 index 0fc5b9dc..00000000 --- a/lib/rules/align/statement-indent.js +++ /dev/null @@ -1,188 +0,0 @@ -const _ = require('lodash') -const BaseChecker = require('./../base-checker') -const { StatementsIndentValidator, Term } = require('./../../common/statements-indent-validator') -const { onSameLine, stopOf, startOf } = require('./../../common/tokens') -const { typeOf } = require('./../../common/tree-traversing') - -const ruleId = 'statement-indent' -const meta = { - type: 'align', - - docs: { - description: 'Statement indentation is incorrect.', - category: 'Style Guide Rules', - examples: { - good: [ - { - description: 'Statements with correct indentation', - code: require('../../../test/fixtures/align/statements_with_correct_indents').join('\n') - } - ], - bad: [ - { - description: 'Statements with incorrect indentation', - code: require('../../../test/fixtures/align/statements_with_incorrect_indents').join('\n') - } - ] - } - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - schema: [] -} - -class StatementIndentChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - enterIfStatement(ctx) { - new StatementsIndentValidator(ctx) - .cases(ifWithoutElse(), ifElse()) - .errorsTo(this._error.bind(this)) - - this.validateBracketsForElseStatement(ctx) - } - - enterWhileStatement(ctx) { - Term.term('while') - .space() - .term('(') - .noSpaces() - .expression() - .noSpaces() - .term(')') - .statement() - .errorsTo(ctx, this._error.bind(this)) - } - - enterDoWhileStatement(ctx) { - Term.term('do') - .statement() - .term('while') - .space() - .term('(') - .noSpaces() - .expression() - .noSpaces() - .term(')') - .noSpaces() - .term(';') - .errorsTo(ctx, this._error.bind(this)) - - this.validateBracketsForDoWhileStatement(ctx) - } - - enterForStatement(ctx) { - new StatementsIndentValidator(ctx).cases(...forLoopAllCases()).errorsTo(this._error.bind(this)) - } - - validateBracketsForElseStatement(ctx) { - const IF_WITH_ELSE_LENGTH = 7 - const STATEMENT_IDX = 4 - const ELSE_IDX = 5 - - if (ctx.children.length === IF_WITH_ELSE_LENGTH) { - this._validateThatOnSameLine(ctx, STATEMENT_IDX, ELSE_IDX) - } - } - - validateBracketsForDoWhileStatement(ctx) { - const STATEMENT_IDX = 1 - const WHILE_IDX = 2 - - this._validateThatOnSameLine(ctx, STATEMENT_IDX, WHILE_IDX) - } - - _validateThatOnSameLine(ctx, blockIndex, nodeIndex) { - const childs = ctx.children - const block = childs[blockIndex] - const node = childs[nodeIndex] - - if (isBlock(block) && !onSameLine(stopOf(block), startOf(node))) { - const nodeText = node.getText() - this._error(node, `${_.capitalize(nodeText)} must be on the same line with close bracket.`) - } - } - - _error(ctx, message) { - this.error(ctx, `Statement indentation is incorrect. ${message}`) - } -} - -function isBlock(ctx) { - let childs = ctx.children - - while (childs && childs.length === 1) { - if (typeOf(childs[0]) === 'block') { - return true - } - - childs = childs[0].children - } - - return false -} - -function forLoopWith(config) { - const { statement, expression1, expression2 } = config - const seq = Term.term('for') - .space() - .term('(') - - if (!(statement && seq.noSpaces().rule('simpleStatement'))) { - seq.term(';') - } - - if (expression1) { - seq.space().expression() - } - seq.term(';') - - if (expression2) { - seq.space().expression() - } - - return seq - .noSpaces() - .term(')') - .statement() -} - -function forLoopAllCases() { - const cases = [] - - for (let i = Math.pow(2, 3) - 1; i >= 0; i -= 1) { - cases.push( - forLoopWith({ - statement: i & 0b001, - expression1: i & 0b010, - expression2: i & 0b100 - }) - ) - } - - return cases -} - -function ifWithoutElse() { - return Term.term('if') - .space() - .term('(') - .noSpaces() - .expression() - .noSpaces() - .term(')') - .statement() -} - -function ifElse() { - return ifWithoutElse() - .term('else') - .statement() -} - -module.exports = StatementIndentChecker diff --git a/lib/rules/best-practises/code-complexity.js b/lib/rules/best-practises/code-complexity.js index eda8476d..980e2d15 100644 --- a/lib/rules/best-practises/code-complexity.js +++ b/lib/rules/best-practises/code-complexity.js @@ -57,79 +57,79 @@ class CodeComplexityChecker extends BaseChecker { this.maxComplexity = (config && config.getNumber('code-complexity', 7)) || 7 } - enterFunctionDefinition(ctx) { - this._attachComplexityScope(ctx) + FunctionDefinition(node) { + this._attachComplexityScope(node) } - enterModifierDefinition(ctx) { - this._attachComplexityScope(ctx) + ModifierDefinition(node) { + this._attachComplexityScope(node) } - enterIfStatement(ctx) { - this._complexityPlusOne(ctx) + IfStatement(node) { + this._complexityPlusOne(node) } - enterWhileStatement(ctx) { - this._complexityPlusOne(ctx) + WhileStatement(node) { + this._complexityPlusOne(node) } - enterDoWhileStatement(ctx) { - this._complexityPlusOne(ctx) + DoWhileStatement(node) { + this._complexityPlusOne(node) } - enterForStatement(ctx) { - this._complexityPlusOne(ctx) + ForStatement(node) { + this._complexityPlusOne(node) } - exitFunctionDefinition(ctx) { - this._verifyComplexityScope(ctx) + 'FunctionDefinition:exit'(node) { + this._verifyComplexityScope(node) } - exitModifierDefinition(ctx) { - this._verifyComplexityScope(ctx) + 'ModifierDefinition:exit'(node) { + this._verifyComplexityScope(node) } - _attachComplexityScope(ctx) { - ComplexityScope.activate(ctx) + _attachComplexityScope(node) { + ComplexityScope.activate(node) } - _complexityPlusOne(ctx) { - const scope = ComplexityScope.of(ctx) + _complexityPlusOne(node) { + const scope = ComplexityScope.of(node) if (scope) { scope.complexityPlusOne() } } - _verifyComplexityScope(ctx) { - const scope = ComplexityScope.of(ctx) + _verifyComplexityScope(node) { + const scope = ComplexityScope.of(node) if (scope && scope.complexity > this.maxComplexity) { - this._error(ctx, scope) + this._error(node, scope) } } - _error(ctx, scope) { + _error(node, scope) { const curComplexity = scope.complexity const maxComplexity = this.maxComplexity const message = `Function has cyclomatic complexity ${curComplexity} but allowed no more than ${maxComplexity}` - this.error(ctx, message) + this.error(node, message) } } class ComplexityScope { - static of(ctx) { - let curCtx = ctx + static of(node) { + let curNode = node - while (curCtx && !curCtx.complexityScope) { - curCtx = curCtx.parentCtx + while (curNode && !curNode.complexityScope) { + curNode = curNode.parent } - return curCtx && curCtx.complexityScope + return curNode && curNode.complexityScope } - static activate(ctx) { - ctx.complexityScope = new ComplexityScope() + static activate(node) { + node.complexityScope = new ComplexityScope() } constructor() { diff --git a/lib/rules/best-practises/function-max-lines.js b/lib/rules/best-practises/function-max-lines.js index bc608220..f8a96e91 100644 --- a/lib/rules/best-practises/function-max-lines.js +++ b/lib/rules/best-practises/function-max-lines.js @@ -1,6 +1,4 @@ -const _ = require('lodash') const BaseChecker = require('./../base-checker') -const { lineOf, stopLine } = require('./../../common/tokens') const { severityDescription } = require('../../doc/utils') const DEFAULT_SEVERITY = 'warn' @@ -46,36 +44,14 @@ class FunctionMaxLinesChecker extends BaseChecker { (config && config.getNumber(ruleId, DEFAULT_MAX_LINES_COUNT)) || DEFAULT_MAX_LINES_COUNT } - enterFunctionDefinition(ctx) { - const block = Block.of(ctx) - - if (block.linesCount() > this.maxLines) { - this._error(block) + FunctionDefinition(node) { + if (this._linesCount(node) > this.maxLines) { + this._error(node) } } - _error(block) { - const linesCount = block.linesCount() - const message = `Function body contains ${linesCount} lines but allowed no more than ${ - this.maxLines - } lines` - this.error(block.ctx, message) - } -} - -class Block { - static of(functionDefinitionCtx) { - const lastNode = _.last(functionDefinitionCtx.children) - return new Block(lastNode) - } - - constructor(ctx) { - this.ctx = ctx - } - - linesCount() { - const ctx = this.ctx - const startStopGap = stopLine(ctx) - lineOf(ctx) + _linesCount(node) { + const startStopGap = node.loc.end.line - node.loc.start.line if (this._isSingleLineBlock(startStopGap)) { return 1 @@ -91,6 +67,14 @@ class Block { _withoutCloseBracket(startStopGap) { return startStopGap - 1 } + + _error(node) { + const linesCount = this._linesCount(node) + const message = `Function body contains ${linesCount} lines but allowed no more than ${ + this.maxLines + } lines` + this.error(node, message) + } } module.exports = FunctionMaxLinesChecker diff --git a/lib/rules/best-practises/index.js b/lib/rules/best-practises/index.js index 043e1215..3c989339 100644 --- a/lib/rules/best-practises/index.js +++ b/lib/rules/best-practises/index.js @@ -7,11 +7,11 @@ const NoUnusedVarsChecker = require('./no-unused-vars') const PayableFallbackChecker = require('./payable-fallback') const ReasonStringChecker = require('./reason-string') -module.exports = function checkers(reporter, config) { +module.exports = function checkers(reporter, config, inputSrc) { return [ new CodeComplexityChecker(reporter, config), new FunctionMaxLinesChecker(reporter, config), - new MaxLineLengthChecker(reporter, config), + new MaxLineLengthChecker(reporter, config, inputSrc), new MaxStatesCountChecker(reporter, config), new NoEmptyBlocksChecker(reporter), new NoUnusedVarsChecker(reporter), diff --git a/lib/rules/best-practises/max-line-length.js b/lib/rules/best-practises/max-line-length.js index 19ac61ea..4b9ec9aa 100644 --- a/lib/rules/best-practises/max-line-length.js +++ b/lib/rules/best-practises/max-line-length.js @@ -39,15 +39,15 @@ const meta = { } class MaxLineLengthChecker extends BaseChecker { - constructor(reporter, config) { + constructor(reporter, config, inputSrc) { super(reporter, ruleId, meta) this.maxLength = (config && config.getNumber(ruleId, 120)) || 120 + this.inputSrc = inputSrc } - enterSourceUnit(ctx) { - const lines = ctx.parser._input.tokenSource._input.strdata.split(lineBreakPattern) - + SourceUnit() { + const lines = this.inputSrc.split(lineBreakPattern) lines.map(line => line.length).forEach(this.validateLineLength.bind(this)) } diff --git a/lib/rules/best-practises/max-states-count.js b/lib/rules/best-practises/max-states-count.js index ba2887cc..0ff928c2 100644 --- a/lib/rules/best-practises/max-states-count.js +++ b/lib/rules/best-practises/max-states-count.js @@ -1,6 +1,5 @@ const _ = require('lodash') const BaseChecker = require('./../base-checker') -const { typeOf } = require('./../../common/tree-traversing') const { severityDescription } = require('../../doc/utils') const ruleId = 'max-states-count' @@ -60,68 +59,23 @@ class MaxStatesCountChecker extends BaseChecker { this.maxStatesCount = (config && config.getNumber('max-states-count', 15)) || 15 } - enterContractDefinition(ctx) { - const countOfVars = new Contract(ctx) - .variableDeclarations() - .filter(curVar => curVar.isNotConstant()).length + ContractDefinition(node) { + const countOfVars = _(node.subNodes) + .filter(({ type }) => type === 'StateVariableDeclaration') + .flatMap(subNode => subNode.variables.filter(variable => !variable.isDeclaredConst)) + .value().length if (countOfVars > this.maxStatesCount) { - this._error(ctx, countOfVars) + this._error(node, countOfVars) } } - _error(ctx, countOfVars) { + _error(node, countOfVars) { const curStatesCount = countOfVars const maxStatesCount = this.maxStatesCount const message = `Contract has ${curStatesCount} states declarations but allowed no more than ${maxStatesCount}` - this.error(ctx, message) - } -} - -class Contract { - constructor(ctx) { - this.ctx = ctx - } - - variableDeclarations() { - return this._children() - .map(i => new ContractPart(i)) - .filter(curPart => curPart.isVarDefinition()) - .map(curPart => curPart.varDefinition()) - } - - _children() { - return this.ctx.children - } -} - -class ContractPart { - constructor(ctx) { - this.ctx = ctx - } - - isVarDefinition() { - const firstChild = this._firstChild() - return typeOf(firstChild) === 'stateVariableDeclaration' - } - - varDefinition() { - return new VarDefinition(this._firstChild()) - } - - _firstChild() { - return _.first(this.ctx.children) - } -} - -class VarDefinition { - constructor(ctx) { - this.ctx = ctx - } - - isNotConstant() { - return !this.ctx.getText().includes('constant') + this.error(node, message) } } diff --git a/lib/rules/best-practises/no-empty-blocks.js b/lib/rules/best-practises/no-empty-blocks.js index 1dba29b9..8dce5c64 100644 --- a/lib/rules/best-practises/no-empty-blocks.js +++ b/lib/rules/best-practises/no-empty-blocks.js @@ -1,11 +1,6 @@ const BaseChecker = require('./../base-checker') -const { typeOf } = require('./../../common/tree-traversing') const { isFallbackFunction, isFunctionDefinition } = require('./../../common/ast-types') -const ONLY_BRACKETS_LENGTH = 2 -const EMPTY_STRUCT_LENGTH = 4 -const EMPTY_ENUM_LENGTH = 4 - const ruleId = 'no-empty-blocks' const meta = { type: 'best-practises', @@ -27,48 +22,50 @@ class NoEmptyBlocksChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitBlock(ctx) { + Block(node) { const isFallbackFunctionBlock = - isFunctionDefinition(ctx.parentCtx) && isFallbackFunction(ctx.parentCtx) + isFunctionDefinition(node.parent) && isFallbackFunction(node.parent) if (isFallbackFunctionBlock) { // ignore empty blocks in fallback functions return } - this._validateChildrenCount(ctx, ONLY_BRACKETS_LENGTH) + this._validateChildrenCount(node, 'statements') } - exitStructDefinition(ctx) { - this._validateChildrenCount(ctx, EMPTY_STRUCT_LENGTH) + StructDefinition(node) { + this._validateChildrenCount(node, 'members') } - exitEnumDefinition(ctx) { - this._validateChildrenCount(ctx, EMPTY_ENUM_LENGTH) + EnumDefinition(node) { + this._validateChildrenCount(node, 'members') } - exitAssemblyBlock(ctx) { - this._validateChildrenCount(ctx, ONLY_BRACKETS_LENGTH) + AssemblyBlock(node) { + this._validateChildrenCount(node, 'operations') } - exitContractDefinition(ctx) { - this._validateContractPartsCount(ctx) + ContractDefinition(node) { + this._validateContractPartsCount(node) } - _validateChildrenCount(ctx, count) { - if (ctx.children && ctx.children.length === count) { - this._error(ctx) + _validateChildrenCount(node, children) { + const blockChildrenCount = node[children].length + + if (blockChildrenCount === 0) { + this._error(node) } } - _validateContractPartsCount(ctx) { - const contractPartCount = ctx.children.filter(i => typeOf(i) === 'contractPart').length + _validateContractPartsCount(node) { + const contractPartCount = node.subNodes.length if (contractPartCount === 0) { - this._error(ctx) + this._error(node) } } - _error(ctx) { - this.warn(ctx, 'Code contains empty block') + _error(node) { + this.warn(node, 'Code contains empty blocks') } } diff --git a/lib/rules/best-practises/no-unused-vars.js b/lib/rules/best-practises/no-unused-vars.js index 591e0ac1..6f922b44 100644 --- a/lib/rules/best-practises/no-unused-vars.js +++ b/lib/rules/best-practises/no-unused-vars.js @@ -1,9 +1,8 @@ const _ = require('lodash') const BaseChecker = require('./../base-checker') -const TreeTraversion = require('./../../common/tree-traversing') +const TreeTraversing = require('./../../common/tree-traversing') -const traversing = new TreeTraversion() -const { typeOf } = TreeTraversion +const traversing = new TreeTraversing() const ruleId = 'no-unused-vars' const meta = { @@ -26,115 +25,105 @@ class NoUnusedVarsChecker extends BaseChecker { super(reporter, ruleId, meta) } - enterFunctionDefinition(ctx) { - const lastNode = _.last(ctx.children) - const funcWithoutBlock = isFuncWithoutBlock(lastNode) - const emptyBlock = isEmptyBlock(lastNode) + FunctionDefinition(node) { + const funcWithoutBlock = isFuncWithoutBlock(node) + const emptyBlock = isEmptyBlock(node) if (!ignoreWhen(funcWithoutBlock, emptyBlock)) { - VarUsageScope.activate(ctx) + VarUsageScope.activate(node) + node.parameters.forEach(parameter => { + this._addVariable(parameter) + }) } } - enterParameter(ctx) { - if (!ignoreWhen(returnParams(ctx))) { - this._addVariable(ctx) - } - } - - enterVariableDeclaration(ctx) { - this._addVariable(ctx) - } - - enterIdentifierList(ctx) { - for (const curId of traversing.findIdentifier(ctx)) { - this._addVariable(curId) - } + VariableDeclarationStatement(node) { + node.variables.forEach(variable => this._addVariable(variable)) } - enterIdentifier(ctx) { - this._trackVarUsage(ctx) + Identifier(node) { + this._trackVarUsage(node) } - enterAssemblyCall(ctx) { - const firstChild = _.first(ctx.children) + AssemblyCall(node) { + const firstChild = node.arguments.length === 0 && node if (firstChild) { + firstChild.name = firstChild.functionName this._trackVarUsage(firstChild) } } - exitFunctionDefinition(ctx) { - if (VarUsageScope.isActivated(ctx)) { - this._reportErrorsFor(ctx) + 'FunctionDefinition:exit'(node) { + if (VarUsageScope.isActivated(node)) { + this._reportErrorsFor(node) } } - _addVariable(ctx) { - const isCtxIdentifier = typeOf(ctx) === 'identifier' - const idNode = isCtxIdentifier ? ctx : findIdentifierInChildren(ctx) - const funcScope = VarUsageScope.of(ctx) + _addVariable(node) { + const idNode = node + const funcScope = VarUsageScope.of(node) if (idNode && funcScope) { - funcScope.addVar(idNode, idNode.getText()) + funcScope.addVar(idNode, idNode.name) } } - _trackVarUsage(ctx) { - const isFunctionName = typeOf(ctx.parentCtx) === 'functionDefinition' - const funcScope = VarUsageScope.of(ctx) + _trackVarUsage(node) { + const isFunctionName = node.type === 'FunctionDefinition' + const funcScope = VarUsageScope.of(node) - if (funcScope && !this._isVarDeclaration(ctx) && !isFunctionName) { - funcScope.trackVarUsage(ctx.getText()) + if (funcScope && !this._isVarDeclaration(node) && !isFunctionName) { + funcScope.trackVarUsage(node.name) } } - _reportErrorsFor(ctx) { - VarUsageScope.of(ctx) + _reportErrorsFor(node) { + VarUsageScope.of(node) .unusedVariables() .forEach(this._error.bind(this)) } - _error({ name, ctx }) { - this.warn(ctx, `Variable "${name}" is unused`) + _error({ name, node }) { + this.warn(node, `Variable "${name}" is unused`) } - _isVarDeclaration(ctx) { - const variableDeclaration = findParentType(ctx, 'variableDeclaration') - const identifierList = findParentType(ctx, 'identifierList') - const parameterList = findParentType(ctx, 'parameterList') + _isVarDeclaration(node) { + const variableDeclaration = findParentType(node, 'VariableDeclaration') + const identifierList = findParentType(node, 'IdentifierList') + const parameterList = findParentType(node, 'ParameterList') return variableDeclaration || identifierList || parameterList } } class VarUsageScope { - static of(ctx) { + static of(node) { let functionNode - if (typeOf(ctx) === 'functionDefinition') { - functionNode = ctx + if (node.type === 'FunctionDefinition') { + functionNode = node } else { - functionNode = findParentType(ctx, 'functionDefinition') + functionNode = findParentType(node, 'FunctionDefinition') } return functionNode && functionNode.funcScope } - static activate(ctx) { - ctx.funcScope = new VarUsageScope() + static activate(node) { + node.funcScope = new VarUsageScope() } - static isActivated(ctx) { - return !!ctx.funcScope + static isActivated(node) { + return !!node.funcScope } constructor() { this.vars = {} } - addVar(ctx, name) { - this.vars[name] = { ctx, usage: 0 } + addVar(node, name) { + this.vars[name] = { node, usage: 0 } } trackVarUsage(name) { @@ -148,38 +137,25 @@ class VarUsageScope { unusedVariables() { return _(this.vars) .pickBy(val => val.usage === 0) - .map((info, varName) => ({ name: varName, ctx: info.ctx })) + .map((info, varName) => ({ name: varName, node: info.node })) .value() } } function isEmptyBlock(node) { - const OPEN_CLOSE_BRACKETS_LENGTH = 2 - return _.size(node.children) === OPEN_CLOSE_BRACKETS_LENGTH + return _.size(node.body && node.body.statements) === 0 } function isFuncWithoutBlock(node) { - return node.getText() === ';' + return node.body === null } function ignoreWhen(...args) { return _.some(args) } -function returnParams(ctx) { - return findParentType(ctx, 'returnParameters') -} - -function typeName(type) { - return type[0].toUpperCase() + type.substring(1) + 'Context' -} - -function findParentType(ctx, type) { - return traversing.findParentType(ctx, typeName(type)) -} - -function findIdentifierInChildren(ctx) { - return traversing.findTypeInChildren(ctx, typeName('identifier')) +function findParentType(node, type) { + return traversing.findParentType(node, type) } module.exports = NoUnusedVarsChecker diff --git a/lib/rules/best-practises/payable-fallback.js b/lib/rules/best-practises/payable-fallback.js index 181d5733..8ed6aeb7 100644 --- a/lib/rules/best-practises/payable-fallback.js +++ b/lib/rules/best-practises/payable-fallback.js @@ -1,4 +1,5 @@ const BaseChecker = require('./../base-checker') +const { isFallbackFunction } = require('../../common/ast-types') const ruleId = 'payable-fallback' const meta = { @@ -35,29 +36,12 @@ class PayableFallbackChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitFunctionDefinition(ctx) { - // 'function' parameterList modifierList returnParameters? ( ';' | block ) ; - const { modifierList } = this._nodes(ctx) - - if (modifierList && !modifierList.getText().includes('payable')) { - this.warn(ctx, 'When fallback is not payable you will not be able to receive ethers') - } - } - - _nodes(ctx) { - if (!this._isFallbackFunction(ctx)) { - return {} + FunctionDefinition(node) { + if (isFallbackFunction(node)) { + if (node.stateMutability !== 'payable') { + this.warn(node, 'When fallback is not payable you will not be able to receive ether') + } } - - return { - modifierList: ctx.children[2] - } - } - - _isFallbackFunction(ctx) { - return ( - ctx.children && ctx.children[1] && ctx.children[1].constructor.name !== 'IdentifierContext' - ) } } diff --git a/lib/rules/best-practises/reason-string.js b/lib/rules/best-practises/reason-string.js index 68e3274a..ff6765c6 100644 --- a/lib/rules/best-practises/reason-string.js +++ b/lib/rules/best-practises/reason-string.js @@ -74,64 +74,46 @@ class ReasonStringChecker extends BaseChecker { DEFAULT_MAX_CHARACTERS_LONG } - enterFunctionCallArguments(ctx) { - if (this.isReasonStringStatement(ctx)) { - const functionParameters = this.getFunctionParameters(ctx) - const functionName = this.getFunctionName(ctx) + FunctionCall(node) { + if (this.isReasonStringStatement(node)) { + const functionParameters = this.getFunctionParameters(node) + const functionName = this.getFunctionName(node) // Throw an error if have no message if ( (functionName === 'revert' && functionParameters.length === 0) || (functionName === 'require' && functionParameters.length <= 1) ) { - this._errorHaveNoMessage(ctx, functionName) + this._errorHaveNoMessage(node, functionName) return } - const lastFunctionParameter = _.last(functionParameters) - const hasReasonMessage = this.hasReasonMessage(lastFunctionParameter) - // If has reason message and is too long, throw an error - if (hasReasonMessage) { - const lastParameterWithoutQuotes = lastFunctionParameter.replace(/['"]+/g, '') - if (lastParameterWithoutQuotes.length > this.maxCharactersLong) { - this._errorMessageIsTooLong(ctx, functionName) - } + const reason = _.last(functionParameters).value + if (reason.length > this.maxCharactersLong) { + this._errorMessageIsTooLong(node, functionName) } } } - isReasonStringStatement(ctx) { - const name = this.getFunctionName(ctx) - return name === 'revert' || name === 'require' - } - - getFunctionName(ctx) { - const parent = ctx.parentCtx - const name = parent.children[0] - return name.getText() + isReasonStringStatement(node) { + return node.expression.name === 'revert' || node.expression.name === 'require' } - getFunctionParameters(ctx) { - const parent = ctx.parentCtx - const nodes = parent.children[2] - const children = nodes.children && nodes.children.length > 0 ? nodes.children[0].children : [] - const parameters = children - .filter(value => value.getText() !== ',') - .map(value => value.getText()) - return parameters + getFunctionName(node) { + return node.expression.name } - hasReasonMessage(str) { - return str.indexOf("'") >= 0 || str.indexOf('"') >= 0 + getFunctionParameters(node) { + return node.arguments } - _errorHaveNoMessage(ctx, key) { - this.error(ctx, `Provide an error message for ${key}`) + _errorHaveNoMessage(node, key) { + this.error(node, `Provide an error message for ${key}`) } - _errorMessageIsTooLong(ctx, key) { - this.error(ctx, `Error message for ${key} is too long`) + _errorMessageIsTooLong(node, key) { + this.error(node, `Error message for ${key} is too long`) } } diff --git a/lib/rules/deprecations/base-deprecation.js b/lib/rules/deprecations/base-deprecation.js index 715309db..60feb4e2 100644 --- a/lib/rules/deprecations/base-deprecation.js +++ b/lib/rules/deprecations/base-deprecation.js @@ -7,9 +7,9 @@ class BaseDeprecation extends BaseChecker { this.deprecationVersion() // to ensure we have one. } - exitPragmaDirective(ctx) { - const pragma = ctx.children[1].getText() - const value = ctx.children[2].getText() + PragmaDirective(node) { + const pragma = node.name + const value = node.value if (pragma === 'solidity') { const contextVersion = value.replace(/[^0-9.]/g, '').split('.') const deprecationAt = this.deprecationVersion().split('.') diff --git a/lib/rules/deprecations/constructor-syntax.js b/lib/rules/deprecations/constructor-syntax.js index 2ce96714..097d2475 100644 --- a/lib/rules/deprecations/constructor-syntax.js +++ b/lib/rules/deprecations/constructor-syntax.js @@ -1,7 +1,4 @@ const BaseDeprecation = require('./base-deprecation') -const TreeTraversing = require('./../../common/tree-traversing') - -const traversing = new TreeTraversing() const ruleId = 'constructor-syntax' const meta = { @@ -28,21 +25,20 @@ class ConstructorSyntax extends BaseDeprecation { return '0.4.22' } - exitFunctionDefinition(ctx) { - if (this.active) { - const contract = traversing.findParentType(ctx, 'ContractDefinitionContext') - const contractName = contract.children[1].getText() - const functionName = ctx.children[1].getText() - if (functionName === contractName) { - this.warn(ctx, 'Constructors should use the new constructor keyword.') - } - } + PragmaDirective(node) { + super.PragmaDirective(node) } - exitConstructorDefinition(ctx) { - if (!this.active) { - const message = 'Constructor keyword not available before 0.4.22 (' + this.version + ')' - this.error(ctx, message) + FunctionDefinition(node) { + if (node.isConstructor) { + if (node.name === null) { + if (!this.active) { + const message = 'Constructor keyword not available before 0.4.22 (' + this.version + ')' + this.error(node, message) + } + } else if (this.active) { + this.warn(node, 'Constructors should use the new constructor keyword.') + } } } } diff --git a/lib/rules/index.js b/lib/rules/index.js index 2d2acdc4..7bd49775 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -3,20 +3,20 @@ const _ = require('lodash') const security = require('./security/index') const naming = require('./naming/index') const order = require('./order/index') -const align = require('./align/index') const bestPractises = require('./best-practises/index') const deprecations = require('./deprecations/index') const miscellaneous = require('./miscellaneous/index') const configObject = require('./../config') const { validSeverityMap } = require('../config/config-validator') -module.exports = function checkers(reporter, configVals, inputSrc, fileName) { +module.exports = function checkers(reporter, configVals, inputSrc, tokens, fileName) { const config = configObject.from(configVals) const { rules = {} } = config const meta = { reporter, config, inputSrc, + tokens, fileName } const plugins = config.plugins || [] @@ -36,16 +36,15 @@ module.exports = function checkers(reporter, configVals, inputSrc, fileName) { } function coreRules(meta) { - const { reporter, config, inputSrc, fileName } = meta + const { reporter, config, inputSrc, tokens } = meta return [ - ...align(reporter, config), - ...bestPractises(reporter, config), - ...deprecations(reporter, config), - ...miscellaneous(reporter, config, inputSrc, fileName), - ...naming(reporter, config), - ...order(reporter, config), - ...security(reporter, config) + ...bestPractises(reporter, config, inputSrc), + ...deprecations(reporter), + ...miscellaneous(reporter, config, tokens), + ...naming(reporter), + ...order(reporter, tokens), + ...security(reporter, config, inputSrc) ] } diff --git a/lib/rules/miscellaneous/index.js b/lib/rules/miscellaneous/index.js index 70a919b1..e9e3b221 100644 --- a/lib/rules/miscellaneous/index.js +++ b/lib/rules/miscellaneous/index.js @@ -1,5 +1,5 @@ const QuotesChecker = require('./quotes') -module.exports = function checkers(reporter, config) { - return [new QuotesChecker(reporter, config)] +module.exports = function checkers(reporter, config, tokens) { + return [new QuotesChecker(reporter, config, tokens)] } diff --git a/lib/rules/miscellaneous/quotes.js b/lib/rules/miscellaneous/quotes.js index eb8c08f7..0323ef2d 100644 --- a/lib/rules/miscellaneous/quotes.js +++ b/lib/rules/miscellaneous/quotes.js @@ -57,42 +57,59 @@ const meta = { } class QuotesChecker extends BaseChecker { - constructor(reporter, config) { + constructor(reporter, config, tokens) { super(reporter, ruleId, meta) + this.tokens = tokens + this.visitedNodes = new Set() const quoteType = config && config.rules && config.rules.quotes && config.rules.quotes[1] this.quoteType = (QUOTE_TYPES.includes(quoteType) && quoteType) || DEFAULT_QUOTES_TYPE this.incorrectQuote = this.quoteType === 'single' ? '"' : "'" } - exitPrimaryExpression(ctx) { - this.validateQuotes(ctx) - } + StringLiteral(node) { + const token = this.tokens.find( + token => + token.loc.start.line === node.loc.start.line && + token.loc.start.column === node.loc.start.column + ) - exitAssemblyLiteral(ctx) { - this.validateQuotes(ctx) + if (token && !this.alreadyVisited(token)) { + this.addVisitedNode(token) + this.validateQuotes(token) + } } - exitImportDirective(ctx) { - const children = ctx.children + ImportDirective(node) { + const token = this.tokens.find( + token => token.loc.start.line === node.loc.start.line && token.value.includes(node.path) + ) - if (children && children.length >= 2) { - this.validateQuotes(children[1]) + if (token) { + this.validateQuotes(token) } + } - for (let i = 0; i < children.length; i += 1) { - const curChild = children[i] + validateQuotes(node) { + if (node.value.startsWith(this.incorrectQuote)) { + this._error(node) + } + } - if (curChild.getText && curChild.getText() === 'from' && i + 1 < children.length) { - this.validateQuotes(children[i + 1]) - } + alreadyVisited({ + loc: { + start: { line, column } } + }) { + return this.visitedNodes.has(`${line}${column}`) } - validateQuotes(ctx) { - if (ctx.getText().startsWith(this.incorrectQuote)) { - this._error(ctx) + addVisitedNode({ + loc: { + start: { line, column } } + }) { + this.visitedNodes.add(`${line}${column}`) } _error(ctx) { diff --git a/lib/rules/naming/const-name-snakecase.js b/lib/rules/naming/const-name-snakecase.js index 6024b104..0cb6bb35 100644 --- a/lib/rules/naming/const-name-snakecase.js +++ b/lib/rules/naming/const-name-snakecase.js @@ -1,9 +1,6 @@ const BaseChecker = require('./../base-checker') -const TreeTraversing = require('./../../common/tree-traversing') const naming = require('./../../common/identifier-naming') -const traversing = new TreeTraversing() - const ruleId = 'const-name-snakecase' const meta = { type: 'naming', @@ -25,29 +22,15 @@ class ConstNameSnakecaseChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitStateVariableDeclaration(ctx) { - const hasConstModifier = ctx.children.some(i => i.getText() === 'constant') - - if (hasConstModifier) { - this.validateConstantName(ctx) + VariableDeclaration(node) { + if (node.isDeclaredConst) { + this.validateConstantName(node) } } - validateConstantName(ctx) { - this._forEachIdentifier(ctx, (curId, text) => { - if (naming.isNotUpperSnakeCase(text)) { - this.error(curId, 'Constant name must be in capitalized SNAKE_CASE') - } - }) - } - - _forEachIdentifier(ctx, callback) { - for (const curId of traversing.findIdentifier(ctx)) { - const text = curId.getText() - - if (callback) { - callback(curId, text) - } + validateConstantName(variable) { + if (naming.isNotUpperSnakeCase(variable.name)) { + this.error(variable, 'Constant name must be in capitalized SNAKE_CASE') } } } diff --git a/lib/rules/naming/contract-name-camelcase.js b/lib/rules/naming/contract-name-camelcase.js index ac920efd..2f483df8 100644 --- a/lib/rules/naming/contract-name-camelcase.js +++ b/lib/rules/naming/contract-name-camelcase.js @@ -22,29 +22,26 @@ class ContractNameCamelcaseChecker extends BaseChecker { super(reporter, ruleId, meta) } - enterContractDefinition(ctx) { - this.validateContractName(ctx) + ContractDefinition(node) { + this.validateName(node) } - enterStructDefinition(ctx) { - this.validateContractName(ctx) + EnumDefinition(node) { + this.validateName(node) } - enterEnumDefinition(ctx) { - this.validateContractName(ctx) + StructDefinition(node) { + this.validateName(node) } - validateContractName(ctx) { - const identifier = ctx.children[1] - const text = identifier.getText() - - if (naming.isNotCamelCase(text)) { - this._error(identifier) + validateName(node) { + if (naming.isNotCamelCase(node.name)) { + this._error(node) } } - _error(ctx) { - this.error(ctx, 'Contract name must be in CamelCase') + _error(node) { + this.error(node, 'Contract name must be in CamelCase') } } diff --git a/lib/rules/naming/event-name-camelcase.js b/lib/rules/naming/event-name-camelcase.js index d5e751ab..361d161c 100644 --- a/lib/rules/naming/event-name-camelcase.js +++ b/lib/rules/naming/event-name-camelcase.js @@ -22,12 +22,9 @@ class EventNameCamelcaseChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitEventDefinition(ctx) { - const identifier = ctx.children[1] - const text = identifier.getText() - - if (naming.isNotCamelCase(text)) { - this.error(ctx, 'Event name must be in CamelCase') + EventDefinition(node) { + if (naming.isNotCamelCase(node.name)) { + this.error(node, 'Event name must be in CamelCase') } } } diff --git a/lib/rules/naming/func-name-mixedcase.js b/lib/rules/naming/func-name-mixedcase.js index 1fa9a91c..70145933 100644 --- a/lib/rules/naming/func-name-mixedcase.js +++ b/lib/rules/naming/func-name-mixedcase.js @@ -1,9 +1,5 @@ const BaseChecker = require('./../base-checker') const naming = require('./../../common/identifier-naming') -const TreeTraversing = require('./../../common/tree-traversing') - -const { typeOf } = TreeTraversing -const traversing = new TreeTraversing() const ruleId = 'func-name-mixedcase' const meta = { @@ -26,24 +22,11 @@ class FuncNameMixedcaseChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitFunctionDefinition(ctx) { - const identifier = ctx.children[1] - - if (typeOf(identifier) === 'identifier') { - const text = identifier.getText() - - if (naming.isNotMixedCase(text) && !this.isConstructor(ctx, text)) { - this.error(ctx, 'Function name must be in mixedCase') - } + FunctionDefinition(node) { + if (naming.isNotMixedCase(node.name) && !node.isConstructor) { + this.error(node, 'Function name must be in mixedCase') } } - - isConstructor(ctx, name) { - const parentDefinition = traversing.findParentType(ctx, 'ContractDefinitionContext') - const contractName = parentDefinition.children[1].getText() - - return contractName === name - } } module.exports = FuncNameMixedcaseChecker diff --git a/lib/rules/naming/func-param-name-mixedcase.js b/lib/rules/naming/func-param-name-mixedcase.js index b1a5eaeb..18718eea 100644 --- a/lib/rules/naming/func-param-name-mixedcase.js +++ b/lib/rules/naming/func-param-name-mixedcase.js @@ -1,13 +1,12 @@ const BaseChecker = require('./../base-checker') const naming = require('./../../common/identifier-naming') -const { typeOf } = require('./../../common/tree-traversing') const ruleId = 'func-param-name-mixedcase' const meta = { type: 'naming', docs: { - description: 'Function name must be in camelCase.', + description: 'Function param name must be in mixedCase', category: 'Style Guide Rules' }, @@ -23,28 +22,16 @@ class FunctionParamNameMixedcaseChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitEventParameter(ctx) { - this.exitParameter(ctx) + EventDefinition(node) { + this.FunctionDefinition(node) } - exitParameter(ctx) { - const identifier = this.findIdentifier(ctx) - - if (identifier && naming.isNotMixedCase(identifier.getText())) { - this._error(identifier) - } - } - - findIdentifier(ctx) { - const children = ctx.children - - const ids = children.filter(i => typeOf(i) === 'identifier') - - return ids.length > 0 && ids[0] - } - - _error(identifier) { - this.error(identifier, 'Function param name must be in mixedCase') + FunctionDefinition(node) { + node.parameters.forEach(parameter => { + if (naming.isNotMixedCase(parameter.name)) { + this.error(parameter, 'Function param name must be in mixedCase') + } + }) } } diff --git a/lib/rules/naming/modifier-name-mixedcase.js b/lib/rules/naming/modifier-name-mixedcase.js index 1aad6c62..f8d3915f 100644 --- a/lib/rules/naming/modifier-name-mixedcase.js +++ b/lib/rules/naming/modifier-name-mixedcase.js @@ -22,12 +22,9 @@ class ModifierNameMixedcase extends BaseChecker { super(reporter, ruleId, meta) } - exitModifierDefinition(ctx) { - const identifier = ctx.children[1] - const text = identifier.getText() - - if (naming.isNotMixedCase(text)) { - this.error(ctx, 'Modifier name must be in mixedCase') + ModifierDefinition(node) { + if (naming.isNotMixedCase(node.name)) { + this.error(node, 'Modifier name must be in mixedCase') } } } diff --git a/lib/rules/naming/use-forbidden-name.js b/lib/rules/naming/use-forbidden-name.js index 459c2a13..a32ff58c 100644 --- a/lib/rules/naming/use-forbidden-name.js +++ b/lib/rules/naming/use-forbidden-name.js @@ -1,6 +1,6 @@ const BaseChecker = require('./../base-checker') -const FROBIDDEN_NAMES = ['I', 'l', 'O'] +const FORBIDDEN_NAMES = ['I', 'l', 'O'] const ruleId = 'use-forbidden-name' const meta = { @@ -23,11 +23,9 @@ class UseForbiddenNameChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitIdentifier(ctx) { - const text = ctx.getText() - - if (FROBIDDEN_NAMES.includes(text)) { - this.error(ctx, "Avoid to use letters 'I', 'l', 'O' as identifiers") + VariableDeclaration(node) { + if (FORBIDDEN_NAMES.includes(node.name)) { + this.error(node, "Avoid to use letters 'I', 'l', 'O' as identifiers") } } } diff --git a/lib/rules/naming/var-name-mixedcase.js b/lib/rules/naming/var-name-mixedcase.js index 3cd77c58..fe593304 100644 --- a/lib/rules/naming/var-name-mixedcase.js +++ b/lib/rules/naming/var-name-mixedcase.js @@ -1,9 +1,6 @@ const BaseChecker = require('./../base-checker') -const TreeTraversing = require('./../../common/tree-traversing') const naming = require('./../../common/identifier-naming') -const traversing = new TreeTraversing() - const ruleId = 'var-name-mixedcase' const meta = { type: 'naming', @@ -25,37 +22,15 @@ class VarNameMixedcaseChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitIdentifierList(ctx) { - this.validateVariablesName(ctx) - } - - exitVariableDeclaration(ctx) { - this.validateVariablesName(ctx) - } - - exitStateVariableDeclaration(ctx) { - const hasConstModifier = ctx.children.some(i => i.getText() === 'constant') - - if (!hasConstModifier) { - this.validateVariablesName(ctx) + VariableDeclaration(node) { + if (!node.isDeclaredConst) { + this.validateVariablesName(node) } } - validateVariablesName(ctx) { - this._forEachIdentifier(ctx, (curId, text) => { - if (naming.isNotMixedCase(text)) { - this.error(curId, 'Variable name must be in mixedCase') - } - }) - } - - _forEachIdentifier(ctx, callback) { - for (const curId of traversing.findIdentifier(ctx)) { - const text = curId.getText() - - if (callback) { - callback(curId, text) - } + validateVariablesName(node) { + if (naming.isNotMixedCase(node.name)) { + this.error(node, 'Variable name must be in mixedCase') } } } diff --git a/lib/rules/order/func-order.js b/lib/rules/order/func-order.js index 8b4a5493..68d2db04 100644 --- a/lib/rules/order/func-order.js +++ b/lib/rules/order/func-order.js @@ -1,5 +1,6 @@ const BaseChecker = require('./../base-checker') const TreeTraversing = require('./../../common/tree-traversing') +const { isFallbackFunction } = require('../../common/ast-types') const traversing = new TreeTraversing() @@ -38,27 +39,27 @@ class FuncOrderChecker extends BaseChecker { super(reporter, ruleId, meta) } - enterContractDefinition(ctx) { - this._assignOrderAnalysisTo(ctx) + ContractDefinition(node) { + this._assignOrderAnalysisTo(node) } - enterStructDefinition(ctx) { - this._assignOrderAnalysisTo(ctx) + StructDefinition(node) { + this._assignOrderAnalysisTo(node) } - _assignOrderAnalysisTo(ctx) { - const nameCtx = ctx.children[1] - ctx.funcOrderAnalysis = new FunctionOrderAnalysis(nameCtx.getText(), this.reporter, this.ruleId) + _assignOrderAnalysisTo(node) { + const nodeName = node.name + node.funcOrderAnalysis = new FunctionOrderAnalysis(nodeName, this.reporter, this.ruleId) } - exitConstructorDefinition(ctx) { - const contract = traversing.findParentType(ctx, 'ContractDefinitionContext') - contract.funcOrderAnalysis.process(ctx) + 'ConstructorDefinition:exit'(node) { + const contract = traversing.findParentType(node, 'ContractDefinition') + contract.funcOrderAnalysis.process(node) } - exitFunctionDefinition(ctx) { - const contract = traversing.findParentType(ctx, 'ContractDefinitionContext') - contract.funcOrderAnalysis.process(ctx) + FunctionDefinition(node) { + const contract = traversing.findParentType(node, 'ContractDefinition') + contract.funcOrderAnalysis.process(node) } } @@ -183,12 +184,11 @@ class FunctionOrderAnalysis { this.curState = STATES.START this.reporter = reporter this.ruleId = ruleId - this.contractName = contractName this.funcTypeParser = new FuncTypeParser(contractName) } - process(ctx) { - const name = this.funcTypeParser.funcType(ctx) + process(node) { + const name = this.funcTypeParser.funcType(node) const newState = this.curState.nextState(name) if (newState) { @@ -196,62 +196,33 @@ class FunctionOrderAnalysis { } else { const afterState = this.curState.after const message = `Function order is incorrect, ${name} function can not go after ${afterState} function.` - this.reporter.error(ctx, this.ruleId, message) + this.reporter.error(node, this.ruleId, message) } } } class FuncTypeParser { - constructor(contractName) { - this.contractName = contractName - } - - funcType(ctx) { - if (ctx.children && ctx.children[0].getText() === 'constructor') { + funcType(node) { + if (node.isConstructor) { return 'constructor' - } else if (ctx.children && ctx.children[1].constructor.name === 'IdentifierContext') { - const funcName = ctx.children[1] - - if (funcName.getText() === this.contractName) { - return 'constructor' - } else { - return this.funcModifierType(ctx) - } - } else { + } else if (isFallbackFunction(node)) { return 'fallback' + } else { + return this.funcModifierType(node) } } - funcModifierType(ctx) { - const modifiers = ctx.children[3] - const text = modifiers.getText() - - if (text.includes('external') && includesAnyOf(text, ['pure', 'view', 'constant'])) { - return 'external_const' - } + funcModifierType(node) { + const modifiers = ['pure', 'view', 'constant'] + const publicVisibility = ['external', 'public'] + const { visibility, stateMutability } = node - if (text.includes('external')) { - return 'external' + if (publicVisibility.includes(visibility) && modifiers.includes(stateMutability)) { + return `${visibility}_const` } - if (text.includes('private')) { - return 'private' - } - - if (text.includes('internal')) { - return 'internal' - } - - if (includesAnyOf(text, ['pure', 'view', 'constant'])) { - return 'public_const' - } else { - return 'public' - } + return visibility } } -function includesAnyOf(text, items) { - return items.map(i => text.includes(i)).some(i => i) -} - module.exports = FuncOrderChecker diff --git a/lib/rules/order/imports-on-top.js b/lib/rules/order/imports-on-top.js index 5b44babf..cc6b9916 100644 --- a/lib/rules/order/imports-on-top.js +++ b/lib/rules/order/imports-on-top.js @@ -1,5 +1,4 @@ const BaseChecker = require('./../base-checker') -const { typeOf } = require('./../../common/tree-traversing') const ruleId = 'imports-on-top' const meta = { @@ -22,23 +21,23 @@ class ImportsOnTopChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitSourceUnit(ctx) { + SourceUnit(node) { let hasContractDef = false - for (let i = 0; ctx.children && i < ctx.children.length; i += 1) { - const curItem = ctx.children[i] + for (let i = 0; node.children && i < node.children.length; i += 1) { + const curItem = node.children[i] - if (typeOf(curItem) === 'contractDefinition') { + if (curItem.type === 'ContractDefinition') { hasContractDef = true } - if (hasContractDef && typeOf(curItem) === 'importDirective') { + if (hasContractDef && curItem.type === 'ImportDirective') { this._error(curItem) } } } - _error(ctx) { - this.error(ctx, 'Import statements must be on top') + _error(node) { + this.error(node, 'Import statements must be on top') } } diff --git a/lib/rules/order/index.js b/lib/rules/order/index.js index 207082dc..0ae90fb2 100644 --- a/lib/rules/order/index.js +++ b/lib/rules/order/index.js @@ -1,15 +1,11 @@ const FuncOrderChecker = require('./func-order') const ImportsOnTopChecker = require('./imports-on-top') -const SeparateByOneLineInContractChecker = require('./separate-by-one-line-in-contract') -const TwoLinesTopLevelSeparatorChecker = require('./two-lines-top-level-separator') const VisibilityModifierOrderChecker = require('./visibility-modifier-order') -module.exports = function order(reporter) { +module.exports = function order(reporter, tokens) { return [ new FuncOrderChecker(reporter), new ImportsOnTopChecker(reporter), - new SeparateByOneLineInContractChecker(reporter), - new TwoLinesTopLevelSeparatorChecker(reporter), - new VisibilityModifierOrderChecker(reporter) + new VisibilityModifierOrderChecker(reporter, tokens) ] } diff --git a/lib/rules/order/separate-by-one-line-in-contract.js b/lib/rules/order/separate-by-one-line-in-contract.js deleted file mode 100644 index 21c9430a..00000000 --- a/lib/rules/order/separate-by-one-line-in-contract.js +++ /dev/null @@ -1,57 +0,0 @@ -const BaseChecker = require('./../base-checker') -const BlankLineCounter = require('./../../common/blank-line-counter') -const { typeOf } = require('./../../common/tree-traversing') - -const ruleId = 'separate-by-one-line-in-contract' -const meta = { - type: 'order', - - docs: { - description: `Definitions inside contract / library must be separated by one line.`, - category: 'Style Guide Rules' - }, - - isDefault: false, - recommended: false, - defaultSetup: 'warn', - - schema: [] -} - -class SeparateByOneLineInContractChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - this.lineCounter = new BlankLineCounter() - } - - enterContractDefinition(ctx) { - const lineCounter = this.lineCounter - lineCounter.calcTokenLines(ctx) - - const items = ctx.children.filter(i => typeOf(i) === 'contractPart') - - for (let i = 0, j = i + 1; j < items.length; i += 1, j += 1) { - const curItem = items[i] - const nextItem = items[j] - const bothPartsIsNotSingleLine = !(this.isSingleLine(curItem) && this.isSingleLine(nextItem)) - - if ( - bothPartsIsNotSingleLine && - lineCounter.countOfEmptyLinesBetween(curItem, nextItem) !== 1 - ) { - this._error(nextItem) - } - } - } - - isSingleLine(item) { - return item.start.line === item.stop.line - } - - _error(ctx) { - const message = 'Definitions inside contract / library must be separated by one line' - this.error(ctx, message) - } -} - -module.exports = SeparateByOneLineInContractChecker diff --git a/lib/rules/order/two-lines-top-level-separator.js b/lib/rules/order/two-lines-top-level-separator.js deleted file mode 100644 index 59f29e8c..00000000 --- a/lib/rules/order/two-lines-top-level-separator.js +++ /dev/null @@ -1,60 +0,0 @@ -const BaseChecker = require('./../base-checker') -const BlankLineCounter = require('./../../common/blank-line-counter') -const { typeOf } = require('./../../common/tree-traversing') - -const ruleId = 'two-lines-top-level-separator' -const meta = { - type: 'order', - - docs: { - description: `Definition must be surrounded with two blank line indent.`, - category: 'Style Guide Rules' - }, - - isDefault: false, - recommended: false, - defaultSetup: 'warn', - - schema: [] -} - -class TwoLinesTopLevelSeparatorChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - this.lineCounter = new BlankLineCounter() - } - - enterSourceUnit(ctx) { - const lineCounter = this.lineCounter - - lineCounter.calcTokenLines(ctx) - - for (let i = 0; ctx.children && i < ctx.children.length; i += 1) { - const prevItemIndex = i - 1 - const prevItem = prevItemIndex >= 0 && ctx.children[prevItemIndex] - - const curItem = ctx.children[i] - - const nextItemIndex = i + 1 - const nextItem = nextItemIndex < ctx.children.length && ctx.children[nextItemIndex] - - if (typeOf(curItem) === 'contractDefinition') { - if (prevItem.stop && lineCounter.countOfEmptyLinesBetween(prevItem, curItem) !== 2) { - this.makeReport(curItem) - continue - } - - if (nextItem.start && lineCounter.countOfEmptyLinesBetween(curItem, nextItem) !== 2) { - this.makeReport(curItem) - } - } - } - } - - makeReport(ctx) { - const message = 'Definition must be surrounded with two blank line indent' - this.error(ctx, message) - } -} - -module.exports = TwoLinesTopLevelSeparatorChecker diff --git a/lib/rules/order/visibility-modifier-order.js b/lib/rules/order/visibility-modifier-order.js index 836c53c4..d1525ef4 100644 --- a/lib/rules/order/visibility-modifier-order.js +++ b/lib/rules/order/visibility-modifier-order.js @@ -31,34 +31,51 @@ const meta = { } class VisibilityModifierOrderChecker extends BaseChecker { - constructor(reporter) { + constructor(reporter, tokens) { super(reporter, ruleId, meta) + this.tokens = tokens } - exitModifierList(ctx) { - if (this.containsVisibilityModifier(ctx)) { - const firstModifier = ctx.children[0] + FunctionDefinition(node) { + if (node.visibility !== 'default' && (node.stateMutability || node.modifiers.length)) { + const functionTokens = [] + const nodeStart = node.loc.start.line + const nodeEnd = node.loc.end.line - if (!this.containsVisibilityModifier(firstModifier)) { - this._error(firstModifier) - } - } - } + for (let i = 0, n = this.tokens.length; i < n; ++i) { + const token = this.tokens[i] + + if (functionTokens.length && token.value === '{') break - containsVisibilityModifier(ctx) { - const text = ctx.getText() + const { + type, + loc: { start } + } = token - const hasInternal = text.includes('internal') - const hasExternal = text.includes('external') - const hasPrivate = text.includes('private') - const hasPublic = text.includes('public') + if ( + nodeStart <= start.line && + start.line <= nodeEnd && + ['Keyword', 'Identifier'].includes(type) + ) { + functionTokens.push(token) + } + } + const visibilityIndex = functionTokens.findIndex(t => t.value === node.visibility) + const stateMutabilityIndex = functionTokens.findIndex(t => t.value === node.stateMutability) + const modifierIndex = functionTokens.findIndex(t => t.value === node.modifiers[0].name) - return hasExternal || hasInternal || hasPrivate || hasPublic + if ( + (stateMutabilityIndex > -1 && visibilityIndex > stateMutabilityIndex) || + (modifierIndex > -1 && visibilityIndex > modifierIndex) + ) { + this._error(functionTokens[visibilityIndex]) + } + } } - _error(ctx) { + _error(node) { const message = 'Visibility modifier must be first in list of modifiers' - this.error(ctx, message) + this.error(node, message) } } diff --git a/lib/rules/security/avoid-call-value.js b/lib/rules/security/avoid-call-value.js index 52fe7067..5009a225 100644 --- a/lib/rules/security/avoid-call-value.js +++ b/lib/rules/security/avoid-call-value.js @@ -21,13 +21,13 @@ class AvoidCallValueChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitExpression(ctx) { - this.validateCallValue(ctx) + MemberAccess(node) { + this.validateCallValue(node) } - validateCallValue(ctx) { - if (ctx.getText().endsWith('.call.value')) { - this.error(ctx, 'Avoid to use ".call.value()()"') + validateCallValue(node) { + if (node.memberName === 'value' && node.expression.memberName === 'call') { + this.error(node, 'Avoid to use ".call.value()()"') } } } diff --git a/lib/rules/security/avoid-low-level-calls.js b/lib/rules/security/avoid-low-level-calls.js index 95245763..107d1c3c 100644 --- a/lib/rules/security/avoid-low-level-calls.js +++ b/lib/rules/security/avoid-low-level-calls.js @@ -30,9 +30,9 @@ class AvoidLowLevelCallsChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitExpression(ctx) { - if (hasMethodCalls(ctx, ['call', 'callcode', 'delegatecall'])) { - this._warn(ctx) + MemberAccess(node) { + if (hasMethodCalls(node, ['call', 'callcode', 'delegatecall'])) { + this._warn(node) } } diff --git a/lib/rules/security/avoid-sha3.js b/lib/rules/security/avoid-sha3.js index 2afcc770..e9d963f4 100644 --- a/lib/rules/security/avoid-sha3.js +++ b/lib/rules/security/avoid-sha3.js @@ -21,11 +21,9 @@ class AvoidSha3Checker extends BaseChecker { super(reporter, ruleId, meta) } - exitIdentifier(ctx) { - const identifier = ctx.Identifier().toString() - - if (identifier === 'sha3') { - this.error(ctx, 'Use "keccak256" instead of deprecated "sha3"') + Identifier(node) { + if (node.name === 'sha3') { + this.error(node, 'Use "keccak256" instead of deprecated "sha3"') } } } diff --git a/lib/rules/security/avoid-suicide.js b/lib/rules/security/avoid-suicide.js index 8d4ad242..dfb9e84d 100644 --- a/lib/rules/security/avoid-suicide.js +++ b/lib/rules/security/avoid-suicide.js @@ -21,11 +21,9 @@ class AvoidSuicideChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitIdentifier(ctx) { - const identifier = ctx.Identifier().toString() - - if (identifier === 'suicide') { - this.error(ctx, 'Use "selfdestruct" instead of deprecated "suicide"') + Identifier(node) { + if (node.name === 'suicide') { + this.error(node, 'Use "selfdestruct" instead of deprecated "suicide"') } } } diff --git a/lib/rules/security/avoid-throw.js b/lib/rules/security/avoid-throw.js index 2b8e9560..931b01fb 100644 --- a/lib/rules/security/avoid-throw.js +++ b/lib/rules/security/avoid-throw.js @@ -21,8 +21,8 @@ class AvoidThrowChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitThrowStatement(ctx) { - this.error(ctx, '"throw" is deprecated, avoid to use it') + ThrowStatement(node) { + this.error(node, '"throw" is deprecated, avoid to use it') } } diff --git a/lib/rules/security/avoid-tx-origin.js b/lib/rules/security/avoid-tx-origin.js index 3ce51e42..38dc8ff1 100644 --- a/lib/rules/security/avoid-tx-origin.js +++ b/lib/rules/security/avoid-tx-origin.js @@ -21,29 +21,10 @@ class AvoidTxOriginChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitExpression(ctx) { - // expression1 '.' expression2 - const { expression1, dot, expression2 } = this._nodes(ctx) - - if (dot && expression1.getText() === 'tx' && expression2.getText() === 'origin') { - this.error(ctx, 'Avoid to use tx.origin') - } - } - - _nodes(ctx) { - if (!this._isDotExpression(ctx)) { - return {} + MemberAccess(node) { + if (node.expression.name === 'tx' && node.memberName === 'origin') { + this.error(node, 'Avoid to use tx.origin') } - - return { - expression1: ctx.children[0], - dot: ctx.children[1], - expression2: ctx.children[2] - } - } - - _isDotExpression(ctx) { - return ctx.children && ctx.children[1] && ctx.children[1].getText() === '.' } } diff --git a/lib/rules/security/check-send-result.js b/lib/rules/security/check-send-result.js index d8ce1514..55435e6d 100644 --- a/lib/rules/security/check-send-result.js +++ b/lib/rules/security/check-send-result.js @@ -38,17 +38,17 @@ class CheckSendResultChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitExpression(ctx) { - this.validateSend(ctx) + MemberAccess(node) { + this.validateSend(node) } - validateSend(ctx) { - if (ctx.getText().includes('.send(')) { - const hasVarDeclaration = traversing.statementNotContains(ctx, 'VariableDeclarationContext') - const hasIfStatement = traversing.statementNotContains(ctx, 'IfStatementContext') + validateSend(node) { + if (node.memberName === 'send') { + const hasVarDeclaration = traversing.statementNotContains(node, 'VariableDeclaration') + const hasIfStatement = traversing.statementNotContains(node, 'IfStatement') if (!hasIfStatement && !hasVarDeclaration) { - this.error(ctx, 'Check result of "send" call') + this.error(node, 'Check result of "send" call') } } } diff --git a/lib/rules/security/compiler-fixed.js b/lib/rules/security/compiler-fixed.js deleted file mode 100644 index 3130840f..00000000 --- a/lib/rules/security/compiler-fixed.js +++ /dev/null @@ -1,31 +0,0 @@ -const BaseChecker = require('./../base-checker') - -const ruleId = 'compiler-fixed' -const meta = { - type: 'security', - - docs: { - description: `Compiler version must be fixed.`, - category: 'Security Rules' - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - deprecated: true, - - schema: [] -} - -class CompilerFixedChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - exitVersionOperator(ctx) { - this.warn(ctx, 'Compiler version must be fixed') - } -} - -module.exports = CompilerFixedChecker diff --git a/lib/rules/security/compiler-gt-0_4.js b/lib/rules/security/compiler-gt-0_4.js deleted file mode 100644 index f4b7dff7..00000000 --- a/lib/rules/security/compiler-gt-0_4.js +++ /dev/null @@ -1,40 +0,0 @@ -const BaseChecker = require('./../base-checker') - -const ruleId = 'compiler-gt-0_4' -const meta = { - type: 'security', - - docs: { - description: `Compiler version must be fixed.`, - category: 'Security Rules' - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - deprecated: true, - - schema: [] -} - -class CompilerGT04Checker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - } - - exitVersionConstraint(ctx) { - const versionNode = - (this.isVersionOperator(ctx.children[0]) && ctx.children[1]) || ctx.children[0] - - if (versionNode.getText() < '0.4') { - this.error(ctx, "Use at least '0.4' compiler version") - } - } - - isVersionOperator(ctx) { - return ctx.constructor.name.includes('VersionOperator') - } -} - -module.exports = CompilerGT04Checker diff --git a/lib/rules/security/compiler-version.js b/lib/rules/security/compiler-version.js index 3df2253b..3882d9bb 100644 --- a/lib/rules/security/compiler-version.js +++ b/lib/rules/security/compiler-version.js @@ -44,23 +44,14 @@ class CompilerVersionChecker extends BaseChecker { this.requirement = (config && config.getString(ruleId, DEFAULT_SEMVER)) || DEFAULT_SEMVER } - exitVersionConstraint(ctx) { - const versionNode = - (this.isVersionOperator(ctx.children[0]) && ctx.children[1]) || ctx.children[0] - - if (!semver.satisfies(versionNode.getText(), this.requirement)) { - this.error( - ctx, - `Compiler version ${versionNode.getText()} does not satisfy the ${ - this.requirement - } semver requirement` + PragmaDirective(node) { + if (!semver.satisfies(semver.minVersion(node.value), this.requirement)) { + this.warn( + node, + `Compiler version ${node.value} does not satisfy the ${this.requirement} semver requirement` ) } } - - isVersionOperator(ctx) { - return ctx.constructor.name.includes('VersionOperator') - } } module.exports = CompilerVersionChecker diff --git a/lib/rules/security/func-visibility.js b/lib/rules/security/func-visibility.js index 78ff4680..1bb1f311 100644 --- a/lib/rules/security/func-visibility.js +++ b/lib/rules/security/func-visibility.js @@ -35,16 +35,9 @@ class FuncVisibilityChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitModifierList(ctx) { - const text = ctx.getText() - - const hasInternal = text.includes('internal') - const hasExternal = text.includes('external') - const hasPrivate = text.includes('private') - const hasPublic = text.includes('public') - - if (!hasExternal && !hasInternal && !hasPrivate && !hasPublic) { - this.warn(ctx, 'Explicitly mark visibility in function') + FunctionDefinition(node) { + if (node.visibility === 'default') { + this.warn(node, 'Explicitly mark visibility in function') } } } diff --git a/lib/rules/security/index.js b/lib/rules/security/index.js index ba32fb5e..5b928b00 100644 --- a/lib/rules/security/index.js +++ b/lib/rules/security/index.js @@ -5,21 +5,18 @@ const AvoidSuicideChecker = require('./avoid-suicide') const AvoidThrowChecker = require('./avoid-throw') const AvoidTxOriginChecker = require('./avoid-tx-origin') const CheckSendResultChecker = require('./check-send-result') -const CompilerFixedChecker = require('./compiler-fixed') const CompilerVersionChecker = require('./compiler-version') -const CompilerGT04Checker = require('./compiler-gt-0_4') const FuncVisibilityChecker = require('./func-visibility') const MarkCallableContractsChecker = require('./mark-callable-contracts') const MultipleSendsChecker = require('./multiple-sends') const NoComplexFallbackChecker = require('./no-complex-fallback') const NoInlineAssemblyChecker = require('./no-inline-assembly') -const NoSimpleEventFuncNameChecker = require('./no-simple-event-func-name') const NotRelyOnBlockHashChecker = require('./not-rely-on-block-hash') const NotRelyOnTimeChecker = require('./not-rely-on-time') const ReentrancyChecker = require('./reentrancy') const StateVisibilityChecker = require('./state-visibility') -module.exports = function security(reporter, config) { +module.exports = function security(reporter, config, inputSrc) { return [ new AvoidCallValueChecker(reporter), new AvoidLowLevelCallsChecker(reporter), @@ -28,18 +25,15 @@ module.exports = function security(reporter, config) { new AvoidThrowChecker(reporter), new AvoidTxOriginChecker(reporter), new CheckSendResultChecker(reporter), - new CompilerFixedChecker(reporter), new CompilerVersionChecker(reporter, config), - new CompilerGT04Checker(reporter), new FuncVisibilityChecker(reporter), new MarkCallableContractsChecker(reporter), new MultipleSendsChecker(reporter), new NoComplexFallbackChecker(reporter), new NoInlineAssemblyChecker(reporter), - new NoSimpleEventFuncNameChecker(reporter), new NotRelyOnBlockHashChecker(reporter), new NotRelyOnTimeChecker(reporter), - new ReentrancyChecker(reporter), + new ReentrancyChecker(reporter, inputSrc), new StateVisibilityChecker(reporter) ] } diff --git a/lib/rules/security/mark-callable-contracts.js b/lib/rules/security/mark-callable-contracts.js index ddbb8eb6..04795e4e 100644 --- a/lib/rules/security/mark-callable-contracts.js +++ b/lib/rules/security/mark-callable-contracts.js @@ -39,77 +39,23 @@ class MarkCallableContractsChecker { this.reporter = reporter this.ruleId = ruleId this.meta = meta - this.nonContractNames = [] } - enterStructDefinition(ctx) { - this.gatherNonContractNames(ctx) - } - - enterEventDefinition(ctx) { - this.gatherNonContractNames(ctx) - } - - enterEnumDefinition(ctx) { - this.gatherNonContractNames(ctx) - } - - enterEnumValue(ctx) { - const enumValue = ctx.getText() - this.nonContractNames.push(enumValue) - } - - exitStateVariableDeclaration(ctx) { - const hasConstModifier = ctx.children.some(i => i.getText() === 'constant') - - if (hasConstModifier) { - this._forEachIdentifier(ctx, (curId, name) => { - if (name) { - this.nonContractNames.push(name) - } - }) - } - } - - exitIdentifier(ctx) { - const identifier = ctx.getText() + Identifier(node) { + const identifier = node.name const isFirstCharUpper = /[A-Z]/.test(identifier[0]) const containsTrustInfo = identifier.toLowerCase().includes('trust') - const isStatement = traversing.findParentType(ctx, 'StatementContext') + const isStatement = traversing.findParentType(node, 'ExpressionStatement') - if ( - isFirstCharUpper && - !containsTrustInfo && - isStatement && - !this.nonContractNames.includes(identifier) - ) { + if (isFirstCharUpper && !containsTrustInfo && isStatement) { this.reporter.addMessage( - ctx.getSourceInterval(), + node.loc, SEVERITY.WARN, 'Explicitly mark all external contracts as trusted or untrusted', this.ruleId ) } } - - gatherNonContractNames(ctx) { - const identifier = ctx.children[1] - const name = identifier.getText() - - if (name) { - this.nonContractNames.push(name) - } - } - - _forEachIdentifier(ctx, callback) { - for (const curId of traversing.findIdentifier(ctx)) { - const text = curId.getText() - - if (callback) { - callback(curId, text) - } - } - } } module.exports = MarkCallableContractsChecker diff --git a/lib/rules/security/multiple-sends.js b/lib/rules/security/multiple-sends.js index 9f1b0dc2..4a39ad7e 100644 --- a/lib/rules/security/multiple-sends.js +++ b/lib/rules/security/multiple-sends.js @@ -25,42 +25,45 @@ class MultipleSendsChecker extends BaseChecker { this.funcDefSet = new Set() } - exitExpression(ctx) { - const isOk = this.validateMultipleSendInFunc(ctx) + MemberAccess(node) { + const isOk = this.validateMultipleSendInFunc(node) + if (isOk) { - this.validateSendCallInLoop(ctx) + this.validateSendCallInLoop(node) } } - validateMultipleSendInFunc(ctx) { - if (ctx.getText().includes('.send(')) { - const curFuncDef = traversing.findParentType(ctx, 'FunctionDefinitionContext') + validateMultipleSendInFunc(node) { + if (node.memberName === 'send') { + const curFuncDef = traversing.findParentType(node, 'FunctionDefinition') - if (curFuncDef && this.funcDefSet.has(curFuncDef)) { - this._error(ctx) - return false - } else { - this.funcDefSet.add(curFuncDef) + if (curFuncDef) { + if (this.funcDefSet.has(curFuncDef.name)) { + this._error(node) + return false + } else { + this.funcDefSet.add(curFuncDef.name) + } } } return true } - validateSendCallInLoop(ctx) { - if (ctx.getText().includes('.send(')) { - const hasForLoop = traversing.findParentType(ctx, 'ForStatementContext') !== null - const hasWhileLoop = traversing.findParentType(ctx, 'WhileStatementContext') !== null - const hasDoWhileLoop = traversing.findParentType(ctx, 'DoWhileStatementContext') !== null + validateSendCallInLoop(node) { + if (node.memberName === 'send') { + const hasForLoop = traversing.findParentType(node, 'ForStatement') !== null + const hasWhileLoop = traversing.findParentType(node, 'WhileStatement') !== null + const hasDoWhileLoop = traversing.findParentType(node, 'DoWhileStatement') !== null if (hasForLoop || hasWhileLoop || hasDoWhileLoop) { - this._error(ctx) + this._error(node) } } } - _error(ctx) { - this.error(ctx, 'Avoid multiple calls of "send" method in single transaction') + _error(node) { + this.error(node, 'Avoid multiple calls of "send" method in single transaction') } } diff --git a/lib/rules/security/no-complex-fallback.js b/lib/rules/security/no-complex-fallback.js index eeaab9be..9bf9345a 100644 --- a/lib/rules/security/no-complex-fallback.js +++ b/lib/rules/security/no-complex-fallback.js @@ -1,7 +1,5 @@ -const TreeTraversing = require('./../../common/tree-traversing') const BaseChecker = require('./../base-checker') - -const traversing = new TreeTraversing() +const { isFallbackFunction } = require('../../common/ast-types') const ruleId = 'no-complex-fallback' const meta = { @@ -24,22 +22,10 @@ class NoComplexFallbackChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitFunctionDefinition(ctx) { - const firstChild = ctx.children[0] - const isFirstChildFunction = firstChild && firstChild.getText() === 'function' - - const secondChild = ctx.children[1] - const isSecondChildParams = - secondChild && secondChild.constructor.name === 'ParameterListContext' - - const isFallbackFunction = isFirstChildFunction && isSecondChildParams - - if (isFallbackFunction) { - const block = traversing.findDownType(ctx, 'BlockContext') - const TERMINAL_NODE_COUNT = 2 - - if (block && block.children.length - TERMINAL_NODE_COUNT >= 2) { - this.warn(ctx, 'Fallback function must be simple') + FunctionDefinition(node) { + if (isFallbackFunction(node)) { + if (node.body.statements.length >= 2) { + this.warn(node, 'Fallback function must be simple') } } } diff --git a/lib/rules/security/no-inline-assembly.js b/lib/rules/security/no-inline-assembly.js index 20ea65fc..b4fc261f 100644 --- a/lib/rules/security/no-inline-assembly.js +++ b/lib/rules/security/no-inline-assembly.js @@ -21,13 +21,12 @@ class NoInlineAssemblyChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitInlineAssemblyStatement(ctx) { - this.error(ctx) + InlineAssemblyStatement(node) { + this.error(node) } - error(ctx) { - const message = 'Avoid to use inline assembly. It is acceptable only in rare cases' - this.warn(ctx, message) + error(node) { + this.warn(node, 'Avoid to use inline assembly. It is acceptable only in rare cases') } } diff --git a/lib/rules/security/no-simple-event-func-name.js b/lib/rules/security/no-simple-event-func-name.js deleted file mode 100644 index f7ef3f28..00000000 --- a/lib/rules/security/no-simple-event-func-name.js +++ /dev/null @@ -1,58 +0,0 @@ -const BaseChecker = require('./../base-checker') - -const ruleId = 'no-simple-event-func-name' -const meta = { - type: 'security', - - docs: { - description: `Event and function names must be different.`, - category: 'Security Rules' - }, - - isDefault: false, - recommended: true, - defaultSetup: 'warn', - - deprecated: true, - - schema: [] -} - -class NoSimpleEventFuncNameChecker extends BaseChecker { - constructor(reporter) { - super(reporter, ruleId, meta) - - this.funcNameMap = {} - this.eventNameMap = {} - } - - exitFunctionDefinition(ctx) { - const secondChild = ctx.children[1] - - if (secondChild.constructor.name === 'IdentifierContext') { - const id = secondChild.getText().toLowerCase() - - if (this.eventNameMap[id]) { - this._error(ctx) - } - - this.funcNameMap[id] = true - } - } - - exitEventDefinition(ctx) { - const id = ctx.children[1].getText().toLowerCase() - - if (this.funcNameMap[id]) { - this._error(ctx) - } - - this.eventNameMap[id] = true - } - - _error(ctx) { - this.warn(ctx, 'Event and function names must be different') - } -} - -module.exports = NoSimpleEventFuncNameChecker diff --git a/lib/rules/security/not-rely-on-block-hash.js b/lib/rules/security/not-rely-on-block-hash.js index 82b35b7d..3b4fedb3 100644 --- a/lib/rules/security/not-rely-on-block-hash.js +++ b/lib/rules/security/not-rely-on-block-hash.js @@ -21,14 +21,14 @@ class NotRelyOnBlockHashChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitExpression(ctx) { - if (ctx.getText() === 'block.blockhash') { - this._warn(ctx) + MemberAccess(node) { + if (node.expression.name === 'block' && node.memberName === 'blockhash') { + this._warn(node) } } - _warn(ctx) { - this.warn(ctx, 'Do not rely on "block.blockhash". Miners can influence its value.') + _warn(node) { + this.warn(node, 'Do not rely on "block.blockhash". Miners can influence its value.') } } diff --git a/lib/rules/security/not-rely-on-time.js b/lib/rules/security/not-rely-on-time.js index ba7661f3..b4a9712b 100644 --- a/lib/rules/security/not-rely-on-time.js +++ b/lib/rules/security/not-rely-on-time.js @@ -21,22 +21,20 @@ class NotRelyOnTimeChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitIdentifier(ctx) { - this._textNotAllowed(ctx, 'now') - } - - exitExpression(ctx) { - this._textNotAllowed(ctx, 'block.timestamp') + Identifier(node) { + if (node.name === 'now') { + this._warn(node) + } } - _textNotAllowed(ctx, avoidedName) { - if (ctx.getText() === avoidedName) { - this._warn(ctx) + MemberAccess(node) { + if (node.expression.name === 'block' && node.memberName === 'timestamp') { + this._warn(node) } } - _warn(ctx) { - this.warn(ctx, 'Avoid to make time-based decisions in your business logic') + _warn(node) { + this.warn(node, 'Avoid to make time-based decisions in your business logic') } } diff --git a/lib/rules/security/reentrancy.js b/lib/rules/security/reentrancy.js index 536dc63c..fc07672e 100644 --- a/lib/rules/security/reentrancy.js +++ b/lib/rules/security/reentrancy.js @@ -2,7 +2,7 @@ const _ = require('lodash') const BaseChecker = require('./../base-checker') const TreeTraversing = require('./../../common/tree-traversing') -const { typeOf, hasMethodCalls, findPropertyInParents } = TreeTraversing +const { hasMethodCalls, findPropertyInParents } = TreeTraversing const ruleId = 'reentrancy' const meta = { @@ -47,38 +47,42 @@ const meta = { } class ReentrancyChecker extends BaseChecker { - constructor(reporter) { + constructor(reporter, inputSrc) { super(reporter, ruleId, meta) + this.inputSrc = inputSrc } - enterContractDefinition(ctx) { - ctx.stateDeclarationScope = new StateDeclarationScope() - const scope = ctx.stateDeclarationScope + ContractDefinition(node) { + node.stateDeclarationScope = new StateDeclarationScope() + const scope = node.stateDeclarationScope - new ContractDefinition(ctx).stateDefinitions().forEach(i => scope.trackStateDeclaration(i)) + new ContractDefinition(node).stateDefinitions().forEach(i => scope.trackStateDeclaration(i)) } - enterFunctionDefinition(ctx) { - ctx.effects = new Effects(StateDeclarationScope.of(ctx)) + FunctionDefinition(node) { + node.effects = new Effects(StateDeclarationScope.of(node)) } - enterExpression(ctx) { - this._checkAssignment(ctx) - this._checkSendCall(ctx) + ExpressionStatement(node) { + this._checkAssignment(node) } - _checkAssignment(ctx) { - const effects = Effects.of(ctx) - const assignOperator = AssignOperator.of(ctx) + MemberAccess(node) { + this._checkSendCall(node) + } + + _checkAssignment(node) { + const assignOperator = AssignOperator.of(node, this.inputSrc) + const effects = Effects.of(node) if (assignOperator && effects && effects.isNotAllowed(assignOperator)) { - this._warn(ctx) + this._warn(node) } } - _checkSendCall(ctx) { - if (hasMethodCalls(ctx, ['send', 'transfer'])) { - Effects.of(ctx).trackTransfer() + _checkSendCall(node) { + if (hasMethodCalls(node, ['send', 'transfer'])) { + Effects.of(node).trackTransfer() } } @@ -88,8 +92,8 @@ class ReentrancyChecker extends BaseChecker { } class Effects { - static of(ctx) { - return findPropertyInParents(ctx, 'effects') + static of(node) { + return findPropertyInParents(node, 'effects') } constructor(statesScope) { @@ -107,8 +111,8 @@ class Effects { } class StateDeclarationScope { - static of(ctx) { - return findPropertyInParents(ctx, 'stateDeclarationScope') + static of(node) { + return findPropertyInParents(node, 'stateDeclarationScope') } constructor() { @@ -122,12 +126,12 @@ class StateDeclarationScope { } class ContractDefinition { - constructor(ctx) { - this.ctx = ctx + constructor(node) { + this.node = node } stateDefinitions() { - return this.ctx.children + return this.node.subNodes .map(i => new ContractPart(i)) .filter(i => i.isStateDefinition()) .map(i => i.getStateDefinition()) @@ -135,12 +139,12 @@ class ContractDefinition { } class ContractPart { - constructor(ctx) { - this.ctx = ctx + constructor(node) { + this.node = node } isStateDefinition() { - return typeOf(this._firstChild()) === 'stateVariableDeclaration' + return this.node.type === 'StateVariableDeclaration' } getStateDefinition() { @@ -148,44 +152,43 @@ class ContractPart { } _firstChild() { - return _.first(this.ctx.children) + return _.first(this.node.variables) } } class StateDefinition { - constructor(ctx) { - this.ctx = ctx + constructor(node) { + this.node = node } stateName() { - return _(this.ctx.children) - .find(i => typeOf(i) === 'identifier') - .getText() + return this.node.name } } class AssignOperator { - static of(ctx) { - const hasThreeItems = _.size(ctx.children) === 3 - const hasAssignOperator = ctx.children[1] && ctx.children[1].getText() === '=' - - if (hasThreeItems && hasAssignOperator) { - return new AssignOperator(ctx) + static of(node, inputSrc) { + if (node.expression.type === 'BinaryOperation') { + return new AssignOperator(node, inputSrc) } } - constructor(ctx) { - this.ctx = ctx + constructor(node, inputSrc) { + this.node = node + this.inputSrc = inputSrc } modifyOneOf(states) { - const assigneeText = this._assignee().getText() + const assigneeText = this._assignee() return states.some(curStateName => assigneeText.includes(curStateName)) } _assignee() { - return this.ctx.children[0] + return this.inputSrc.slice( + this.node.expression.left.range[0], + this.node.expression.left.range[1] + 1 + ) } } diff --git a/lib/rules/security/state-visibility.js b/lib/rules/security/state-visibility.js index 7822ae2d..07d19df9 100644 --- a/lib/rules/security/state-visibility.js +++ b/lib/rules/security/state-visibility.js @@ -21,15 +21,9 @@ class StateVisibilityChecker extends BaseChecker { super(reporter, ruleId, meta) } - exitStateVariableDeclaration(ctx) { - const text = ctx.getText() - - const hasInternal = text.includes('internal') - const hasPrivate = text.includes('private') - const hasPublic = text.includes('public') - - if (!hasInternal && !hasPrivate && !hasPublic) { - this.warn(ctx, 'Explicitly mark visibility of state') + StateVariableDeclaration(node) { + if (node.variables.some(({ visibility }) => visibility === 'default')) { + this.warn(node, 'Explicitly mark visibility of state') } } } diff --git a/lib/tree-listener.js b/lib/tree-listener.js index 233b6db1..6dbe4e49 100644 --- a/lib/tree-listener.js +++ b/lib/tree-listener.js @@ -1,19 +1,14 @@ -const SolidityListener = require('./grammar/SolidityListener').SolidityListener - -class SecurityErrorListener extends SolidityListener { +class SecurityErrorListener { constructor(checkers) { - super() this.listenersMap = {} checkers.forEach(i => this.addChecker(i)) } addChecker(newChecker) { - const listenerMethods = Object.getOwnPropertyNames(SolidityListener.prototype) + const listenerMethods = Object.getOwnPropertyNames(newChecker.constructor.prototype) - const usedListenerMethods = listenerMethods - .filter(i => i !== 'constructor') - .filter(i => !!newChecker[i]) + const usedListenerMethods = listenerMethods.filter(i => /^[A-Z]/.test(i)) usedListenerMethods.forEach(methodName => this.addNewListener(methodName, newChecker)) diff --git a/package-lock.json b/package-lock.json index 1d542199..202f7a47 100644 --- a/package-lock.json +++ b/package-lock.json @@ -915,6 +915,11 @@ "integrity": "sha1-8S4PPF13sLHN2RRpQuTpbB5N1SU=", "dev": true }, + "ast-parents": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/ast-parents/-/ast-parents-0.0.1.tgz", + "integrity": "sha1-UI/Q8F0MSHddnszaLhdEIyYejdM=" + }, "asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", @@ -4010,6 +4015,10 @@ "is-fullwidth-code-point": "^2.0.0" } }, + "solidity-parser-antlr": { + "version": "git+https://github.com/fvictorio/solidity-parser-antlr.git#a237cb7417399eed129958ff89ea30ea87bd4b90", + "from": "git+https://github.com/fvictorio/solidity-parser-antlr.git#stable" + }, "source-map": { "version": "0.5.7", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.5.7.tgz", diff --git a/package.json b/package.json index 4369890d..cf4161d6 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "dependencies": { "ajv": "^6.6.1", "antlr4": "4.7.1", + "ast-parents": "0.0.1", "chalk": "^2.4.2", "commander": "2.18.0", "cosmiconfig": "^5.0.7", @@ -41,7 +42,8 @@ "ignore": "^4.0.6", "js-yaml": "^3.12.0", "lodash": "^4.17.11", - "semver": "^6.3.0" + "semver": "^6.3.0", + "solidity-parser-antlr": "https://github.com/fvictorio/solidity-parser-antlr#stable" }, "devDependencies": { "@stryker-mutator/core": "^2.3.0", diff --git a/test/common/contract-builder.js b/test/common/contract-builder.js index f830d262..317b022c 100644 --- a/test/common/contract-builder.js +++ b/test/common/contract-builder.js @@ -21,7 +21,7 @@ function funcWith(statements) { function modifierWith(statements) { return contractWith(` - modifier b() public { + modifier b() { ${statements} } `) diff --git a/test/rules/align/array-declaration-spaces.js b/test/rules/align/array-declaration-spaces.js deleted file mode 100644 index 88053e72..00000000 --- a/test/rules/align/array-declaration-spaces.js +++ /dev/null @@ -1,23 +0,0 @@ -const assert = require('assert') -const { assertErrorMessage, assertNoErrors } = require('./../../common/asserts') -const linter = require('./../../../lib/index') - -describe('Linter - array-declaration-spaces', () => { - it('should raise error when array declaration has spaces', () => { - const code = require('../../fixtures/align/array_declaration_with_spaces') - - const report = linter.processStr(code, { rules: { 'array-declaration-spaces': 'error' } }) - - assert.equal(report.errorCount, 2) - assertErrorMessage(report, 0, 'Array declaration') - assertErrorMessage(report, 1, 'Array declaration') - }) - - it('should not raise error for array declaration', () => { - const code = require('../../fixtures/align/array_declaration') - - const report = linter.processStr(code, { rules: { 'array-declaration-spaces': 'error' } }) - - assertNoErrors(report) - }) -}) diff --git a/test/rules/align/bracket-align.js b/test/rules/align/bracket-align.js deleted file mode 100644 index 5a7c2035..00000000 --- a/test/rules/align/bracket-align.js +++ /dev/null @@ -1,33 +0,0 @@ -const assert = require('assert') -const { assertErrorMessage, assertNoErrors, assertErrorCount } = require('./../../common/asserts') -const linter = require('./../../../lib/index') - -describe('Linter - bracket-align', () => { - it('should raise error when bracket incorrect aligned', () => { - const code = require('../../fixtures/align/incorrectly_aligned_forloop_brackets') - - const report = linter.processStr(code, { - rules: { 'bracket-align': 'error' } - }) - - assert.equal(report.errorCount, 1) - assertErrorMessage(report, 0, 'Open bracket') - }) - - it('should not raise error when function bracket correct aligned', () => { - const code = require('../../fixtures/align/correctly_aligned_function_brackets') - - const report = linter.processStr(code, { rules: { 'bracket-align': 'error' } }) - - assertNoErrors(report) - }) - - it('should raise error when function bracket incorrect aligned', () => { - const code = require('../../fixtures/align/incorrectly_aligned_function_brackets') - - const report = linter.processStr(code, { rules: { 'bracket-align': 'error' } }) - - assertErrorCount(report, 1) - assertErrorMessage(report, 'bracket') - }) -}) diff --git a/test/rules/align/expression-indent.js b/test/rules/align/expression-indent.js deleted file mode 100644 index 52198867..00000000 --- a/test/rules/align/expression-indent.js +++ /dev/null @@ -1,35 +0,0 @@ -const assert = require('assert') -const linter = require('./../../../lib/index') -const { funcWith } = require('./../../common/contract-builder') -const { assertNoErrors } = require('./../../common/asserts') - -describe('Linter - expression-indent', () => { - describe('Incorrect expression-indent', () => { - require('../../fixtures/align/expressions_with_incorrect_indents').forEach(curExpr => - it(`should raise expression indentation error for ${curExpr}`, () => { - const code = funcWith(curExpr + ';') - - const report = linter.processStr(code, { - rules: { 'expression-indent': 'error' } - }) - - assert.equal(report.errorCount, 1) - assert.ok(report.messages[0].message.includes('Expression indentation is incorrect')) - }) - ) - }) - - describe('Correct expression-indent', () => { - require('../../fixtures/align/expressions_with_correct_indents').forEach(curExpr => - it(`should not raise expression indentation error for ${curExpr}`, () => { - const code = funcWith(curExpr + ';') - - const report = linter.processStr(code, { - rules: { 'expression-indent': 'error' } - }) - - assertNoErrors(report, 0) - }) - ) - }) -}) diff --git a/test/rules/align/indent.js b/test/rules/align/indent.js deleted file mode 100644 index 3a5d1f43..00000000 --- a/test/rules/align/indent.js +++ /dev/null @@ -1,288 +0,0 @@ -const assert = require('assert') -const { assertErrorMessage, assertNoErrors } = require('./../../common/asserts') -const linter = require('./../../../lib/index') -const { contractWith, multiLine } = require('./../../common/contract-builder') - -describe('Linter - indent', () => { - it('should raise error when line indent is incorrect with tabs', () => { - const code = '\t\timport "lib.sol";' - - const report = linter.processStr(code, { rules: { indent: ['error', 'tabs'] } }) - - assert.equal(report.errorCount, 1) - assertErrorMessage(report, 0, 'indent') - }) - - it('should raise error when line indent is incorrect with spaces', () => { - const code = require('../../fixtures/align/incorrectly_indented_contract') - - const report = linter.processStr(code, { rules: { indent: ['error', 'spaces'] } }) - - assert.equal(report.errorCount, 3) - assertErrorMessage(report, 0, '0') - assertErrorMessage(report, 1, '4') - assertErrorMessage(report, 2, '0') - }) - - it('should not raise error for multiline multi variable functions with additional indent', () => { - const code = require('../../fixtures/align/correctly_indented_contract') - - const report = linter.processStr(code, { rules: { indent: ['error', 'spaces'] } }) - - assert.equal(report.errorCount, 0) - }) - - it('should not raise error for multiline multi variable functions with no additional indent', () => { - const code = multiLine( - 'contract A { ', - ' function a() public view returns (uint, uint) {', - ' return (1, 2); ', - ' } ', - ' ', - ' function b() public view returns (uint, uint) {', - ' ( ', - ' uint c, ', - ' uint d ', - ' ) = a(); ', - ' return (c, d); ', - ' } ', - '} ' - ) - - const report = linter.processStr(code, { rules: { indent: ['error', 'spaces'] } }) - - assert.equal(report.errorCount, 0) - }) - - it('should raise error when line indent is incorrect for function', () => { - const code = multiLine( - ' contract A { ', - ' uint private a; ', - ' function A() private { ', - ' } ', - ' } ' - ) - - const report = linter.processStr(code, { - rules: { indent: ['error', 'spaces'] } - }) - - assert.equal(report.errorCount, 5) - assertErrorMessage(report, 0, 'Expected indentation of 0') - assertErrorMessage(report, 1, 'Expected indentation of 4') - assertErrorMessage(report, 2, 'Expected indentation of 4') - assertErrorMessage(report, 3, 'Expected indentation of 4') - assertErrorMessage(report, 4, 'Expected indentation of 0') - }) - - it('should raise error when line indent is incorrect for function with for loop', () => { - const code = multiLine( - ' ', // 1 - ' contract A { ', // 2 - ' uint private a; ', // 3 - ' function A() private { ', // 4 - ' for (uint a; a < b; a += 1) ', // 5 - ' break; ', // 6 - ' } ', // 7 - ' } ' // 8 - ) - - const report = linter.processStr(code, { - rules: { indent: ['error', 'spaces'] } - }) - - assert.equal(report.errorCount, 6) - assertErrorMessage(report, 0, 'Expected indentation of 0') - assertErrorMessage(report, 1, 'Expected indentation of 4') - assertErrorMessage(report, 2, 'Expected indentation of 4') - assertErrorMessage(report, 3, 'Expected indentation of 8') - assertErrorMessage(report, 4, 'Expected indentation of 4') - assertErrorMessage(report, 5, 'Expected indentation of 0 spaces') - }) - - it('should raise error when line indent is incorrect for function with for while loop', () => { - const code = multiLine( - ' ', // 1 - ' contract A { ', // 2 - ' uint private a; ', // 3 - ' function A() private { ', // 4 - ' while (a < b) ', // 5 - ' return; ', // 6 - ' } ', // 7 - ' } ' // 8 - ) - - const report = linter.processStr(code, { - rules: { indent: ['error', 'spaces'] } - }) - - assert.equal(report.errorCount, 7) - assertErrorMessage(report, 0, 'Expected indentation of 0 spaces') - assertErrorMessage(report, 1, 'Expected indentation of 4') - assertErrorMessage(report, 2, 'Expected indentation of 4') - assertErrorMessage(report, 3, 'Expected indentation of 8') - assertErrorMessage(report, 4, 'Expected indentation of 12') - assertErrorMessage(report, 5, 'Expected indentation of 4') - assertErrorMessage(report, 6, 'Expected indentation of 0') - }) - - it('should raise error when line indent is incorrect for function with for if statement', () => { - const code = multiLine( - ' ', // 1 - ' contract A { ', // 2 - ' uint private a; ', // 3 - ' function A() private { ', // 4 - ' if (a < b) { ', // 5 - ' a += 1; ', // 6 - ' b -= 1; ', // 7 - ' continue; ', // 8 - ' } ', // 9 - ' } ', // 10 - ' } ' // 11 - ) - - const report = linter.processStr(code, { - rules: { indent: ['error', 'spaces'] } - }) - - assert.equal(report.errorCount, 7) - assertErrorMessage(report, 0, 'Expected indentation of 0 spaces') - assertErrorMessage(report, 1, 'Expected indentation of 4') - assertErrorMessage(report, 2, 'Expected indentation of 4') - assertErrorMessage(report, 3, 'Expected indentation of 8') - assertErrorMessage(report, 4, 'Expected indentation of 12') - assertErrorMessage(report, 5, 'Expected indentation of 4') - assertErrorMessage(report, 6, 'Expected indentation of 0') - }) - - it('should not raise error when line indent is correct for function with for if-else statement with spaces', () => { - const code = multiLine( - ' ', // 1 - 'contract A { ', // 2 - ' function A() private { ', // 3 - ' if (a < b) { ', // 4 - ' a += 1; ', // 5 - ' } else { ', // 6 - ' b -= 1; ', // 7 - ' } ', // 8 - ' } ', // 9 - '} ' // 10 - ) - - const report = linter.processStr(code, { - rules: { indent: ['error', 'spaces'] } - }) - - assertNoErrors(report) - }) - - it('should raise error when line indent is not correct for function with for assembly statement', () => { - const code = multiLine( - ' ', // 1 - 'contract A { ', // 2 - ' function A() private { ', // 3 - ' assembly { ', // 4 - ' {} ', // 5 - ' } ', // 6 - ' } ', // 7 - '} ' // 8 - ) - - const report = linter.processStr(code, { - rules: { - indent: ['error', 'spaces'] - } - }) - - assert.equal(report.errorCount, 1) - assertErrorMessage(report, 0, 'Indentation is incorrect') - }) - - it('should not raise error when indent is correct for function with non single line header', () => { - const code = multiLine( - ' ', // 1 - 'contract A { ', // 2 - ' function A( ', // 3 - ' ) ', // 4 - ' private ', // 5 - ' { ', // 6 - ' { ', // 7 - ' ', // 8 - ' } ', // 9 - ' } ', // 10 - '} ' // 11 - ) - - const report = linter.processStr(code, { - rules: { - indent: ['error', 'spaces'] - } - }) - - assert.equal(report.errorCount, 4) - assertErrorMessage(report, 0, 'Expected indentation of 4') - assertErrorMessage(report, 1, 'Indentation is incorrect') - assertErrorMessage(report, 2, 'Indentation is incorrect') - assertErrorMessage(report, 3, 'Expected indentation of 8') - }) - - it('should not raise error when line indent is correct for function with for if-else statement', () => { - const code = multiLine( - ' ', // 1 - 'contract A { ', // 2 - ' function A() private { ', // 3 - ' if ( ', // 4 - ' a < b ', // 5 - ' ) { ', // 6 - ' a += 1; ', // 7 - ' } else { ', // 8 - ' b -= 1; ', // 9 - ' } ', // 10 - ' } ', // 11 - '} ' // 12 - ) - - const report = linter.processStr(code, { - rules: { indent: ['error', 'spaces'] } - }) - - assertNoErrors(report) - }) - - it('should not raise error for custom configured indent rules', () => { - const code = multiLine( - '', - 'contract A { ', - '\tuint private a = 0; ', - '\tfunction A() { ', - '\t\t\tuint a = 5; ', - '\t} ', - '} ' - ) - - const report = linter.processStr(code, { - rules: { - indent: ['warn', 'tabs'] - } - }) - - assert.equal(report.warningCount, 1) - assertErrorMessage(report, 0, 'Expected indentation of 2 tabs') - }) - - it('should not raise error when function bracket correct aligned', () => { - const code = contractWith(` - function a ( - uint a - ) - public - { - continue; - } - `) - - const report = linter.processStr(code, { rules: { indent: ['warn', 'tabs'] } }) - - assertNoErrors(report) - }) -}) diff --git a/test/rules/align/no-mix-tabs-and-spaces.js b/test/rules/align/no-mix-tabs-and-spaces.js deleted file mode 100644 index 5643f1e8..00000000 --- a/test/rules/align/no-mix-tabs-and-spaces.js +++ /dev/null @@ -1,14 +0,0 @@ -const assert = require('assert') -const { assertErrorMessage } = require('./../../common/asserts') -const linter = require('./../../../lib/index') - -describe('Linter - no-mix-tabs-and-spaces', () => { - it('should raise error about mixed tabs and spaces', () => { - const code = require('../../fixtures/align/expression_with_mixed_tabs_and_spaces') - - const report = linter.processStr(code, { rules: { 'no-mix-tabs-and-spaces': 'error' } }) - - assert.equal(report.errorCount, 1) - assertErrorMessage(report, 0, 'Mixed tabs and spaces') - }) -}) diff --git a/test/rules/align/no-spaces-before-semicolon.js b/test/rules/align/no-spaces-before-semicolon.js deleted file mode 100644 index 8ae2ed2f..00000000 --- a/test/rules/align/no-spaces-before-semicolon.js +++ /dev/null @@ -1,30 +0,0 @@ -const { assertErrorMessage, assertNoErrors, assertErrorCount } = require('./../../common/asserts') -const { funcWith } = require('../../common/contract-builder') -const linter = require('./../../../lib/index') - -describe('Linter - no-spaces-before-semicolon', () => { - require('../../fixtures/align/expressions_with_incorrect_semicolon_align') - .map(exp => funcWith(exp)) - .forEach(curExpr => - it('should raise error when semicolon incorrect aligned', () => { - const report = linter.processStr(curExpr, { - rules: { 'no-spaces-before-semicolon': 'error' } - }) - - assertErrorCount(report, 1) - assertErrorMessage(report, 'Semicolon must not have spaces before') - }) - ) - - require('../../fixtures/align/expressions_with_correct_semicolon_align') - .map(exp => funcWith(exp)) - .forEach(curExpr => - it('should raise error when semicolon incorrect aligned', () => { - const report = linter.processStr(curExpr, { - rules: { 'no-spaces-before-semicolon': 'error' } - }) - - assertNoErrors(report) - }) - ) -}) diff --git a/test/rules/align/space-after-comma.js b/test/rules/align/space-after-comma.js deleted file mode 100644 index 127b3152..00000000 --- a/test/rules/align/space-after-comma.js +++ /dev/null @@ -1,25 +0,0 @@ -const { assertErrorMessage, assertNoErrors, assertErrorCount } = require('./../../common/asserts') -const linter = require('./../../../lib/index') - -describe('Linter - space-after-comma', () => { - require('../../fixtures/align/expressions_with_incorrect_comma_align').forEach(curExpr => - it('should raise error when comma incorrect aligned', () => { - const report = linter.processStr(curExpr, { - rules: { 'space-after-comma': 'error' } - }) - - assertErrorCount(report, 1) - assertErrorMessage(report, 'must be separated') - }) - ) - - require('../../fixtures/align/expressions_with_correct_comma_align').forEach(curExpr => - it('should raise error when comma incorrect aligned', () => { - const report = linter.processStr(curExpr, { - rules: { 'space-after-comma': 'error' } - }) - - assertNoErrors(report) - }) - ) -}) diff --git a/test/rules/align/statement-indent.js b/test/rules/align/statement-indent.js deleted file mode 100644 index 5f05f57a..00000000 --- a/test/rules/align/statement-indent.js +++ /dev/null @@ -1,38 +0,0 @@ -const assert = require('assert') -const linter = require('./../../../lib/index') -const { funcWith } = require('./../../common/contract-builder') - -describe('Linter - statement-indent', () => { - describe('Incorrect Statements statement-indent', () => { - require('../../fixtures/align/statements_with_incorrect_indents').forEach(curStatement => - it(`${label(curStatement)} should raise statement indentation error`, () => { - const code = funcWith(curStatement) - - const report = linter.processStr(code, { - rules: { 'statement-indent': 'error' } - }) - - assert.equal(report.errorCount, 1) - assert.ok(report.messages[0].message.includes('Statement indentation is incorrect')) - }) - ) - }) - - describe('Correct Statements statement-indent', () => { - require('../../fixtures/align/statements_with_correct_indents').forEach(curStatement => - it(`${label(curStatement)} should not raise statement indentation error`, () => { - const code = funcWith(curStatement) - - const report = linter.processStr(code, { - rules: { 'statement-indent': 'error' } - }) - - assert.equal(report.errorCount, 0) - }) - ) - }) - - function label(data) { - return data.split('\n')[0] - } -}) diff --git a/test/rules/best-practises/max-line-length.js b/test/rules/best-practises/max-line-length.js index f1ed7070..0f997b0d 100644 --- a/test/rules/best-practises/max-line-length.js +++ b/test/rules/best-practises/max-line-length.js @@ -1,18 +1,29 @@ const assert = require('assert') const { assertErrorMessage, assertNoErrors } = require('./../../common/asserts') +const { contractWith } = require('./../../common/contract-builder') const linter = require('./../../../lib/index') describe('Linter - max-line-length', () => { it('should raise error when line length exceed 120', () => { const code = ' '.repeat(121) - const report = linter.processStr(code, { + const report = linter.processStr(contractWith(code), { rules: { 'max-line-length': 'error' } }) assert.equal(report.errorCount, 1) assertErrorMessage(report, 0, 'Line length must be no more than') }) + it('should not raise error with an empty file', () => { + const code = ' '.repeat(121) + + const report = linter.processStr(code, { + rules: { 'max-line-length': 'error' } + }) + + assertNoErrors(report) + }) + it('should not raise error when line length exceed 120 and custom config provided', () => { const code = ' '.repeat(130) diff --git a/test/rules/best-practises/no-unused-vars.js b/test/rules/best-practises/no-unused-vars.js index e286fe13..22964903 100644 --- a/test/rules/best-practises/no-unused-vars.js +++ b/test/rules/best-practises/no-unused-vars.js @@ -1,4 +1,3 @@ -const assert = require('assert') const { assertNoWarnings, assertErrorMessage, assertWarnsCount } = require('./../../common/asserts') const linter = require('./../../../lib/index') const { contractWith, funcWith, multiLine } = require('./../../common/contract-builder') @@ -28,7 +27,13 @@ describe('Linter - no-unused-vars', () => { contractWith('function a(uint d) public returns (uint c) { }'), contractWith('function a(uint a, uint c) public returns (uint c);'), contractWith( - 'function a(address a) internal { assembly { t := eq(a, and(mask, calldataload(4))) } }' + multiLine( + 'function a(address a) internal {', + ' assembly {', + ' let t := eq(a, and(mask, calldataload(4)))', + ' }', + '}' + ) ), contractWith( multiLine( @@ -66,60 +71,6 @@ describe('Linter - no-unused-vars', () => { }) ) - it('should raise incorrect var name error', () => { - const code = funcWith('var (a, B);') - - const report = linter.processStr(code, { - rules: { 'no-unused-vars': 'error', 'var-name-mixedcase': 'error' } - }) - - assert.ok(report.errorCount > 0) - assert.ok(report.messages.map(i => i.message).some(i => i.includes('name'))) - }) - - it('should raise incorrect var name error for typed declaration', () => { - const code = funcWith('uint B = 1;') - - const report = linter.processStr(code, { - rules: { 'no-unused-vars': 'error', 'var-name-mixedcase': 'error' } - }) - - assert.ok(report.errorCount > 0) - assert.ok(report.messages.map(i => i.message).some(i => i.includes('name'))) - }) - - it('should raise incorrect var name error for state declaration', () => { - const code = contractWith('uint32 private D = 10;') - - const report = linter.processStr(code, { - rules: { 'no-unused-vars': 'error', 'var-name-mixedcase': 'error' } - }) - - assert.equal(report.errorCount, 1) - assert.ok(report.messages[0].message.includes('Variable name')) - }) - - it('should not raise var name error for constants', () => { - const code = contractWith('uint32 private constant D = 10;') - - const report = linter.processStr(code, { - rules: { 'no-unused-vars': 'error', 'var-name-mixedcase': 'error' } - }) - - assert.equal(report.errorCount, 0) - }) - - it('should raise forbidden name error', () => { - const code = funcWith('uint l = 0;') - - const report = linter.processStr(code, { - rules: { 'no-unused-vars': 'error', 'use-forbidden-name': 'error' } - }) - - assert.equal(report.errorCount, 2) - assert.ok(report.messages[0].message.includes('Avoid to use')) - }) - function label(data) { const items = data.split('\n') const lastItemIndex = items.length - 1 diff --git a/test/rules/miscellaneous/quotes.js b/test/rules/miscellaneous/quotes.js index f528c98b..67e7182a 100644 --- a/test/rules/miscellaneous/quotes.js +++ b/test/rules/miscellaneous/quotes.js @@ -31,16 +31,7 @@ describe('Linter - quotes', () => { assert.ok(report.messages[0].message.includes('double quotes')) }) - it('should raise quotes error in import for custom rules I', () => { - const code = 'import * from "lib.sol";' - - const report = linter.processStr(code, { rules: { quotes: ['error', 'single'] } }) - - assert.equal(report.errorCount, 1) - assert.ok(report.messages[0].message.includes('single quotes')) - }) - - it('should raise quotes error in import for custom rules II', () => { + it('should raise quotes error in import for custom rules', () => { const code = 'import * from "lib.sol";' const report = linter.processStr(code, { rules: { quotes: ['error', 'single'] } }) @@ -86,7 +77,7 @@ describe('Linter - quotes', () => { assert.equal(report.filePath, filePath) }) - it('should raise an one error', () => { + it('should raise one error', () => { const filePath = storeAsFile(contractWith("string private a = 'test';")) const reports = linter.processPath(filePath, { diff --git a/test/rules/order/separate-by-one-line-in-contract.js b/test/rules/order/separate-by-one-line-in-contract.js deleted file mode 100644 index 84f9ad35..00000000 --- a/test/rules/order/separate-by-one-line-in-contract.js +++ /dev/null @@ -1,55 +0,0 @@ -const assert = require('assert') -const { assertErrorMessage, assertNoErrors } = require('./../../common/asserts') -const linter = require('./../../../lib/index') -const { contractWith } = require('./../../common/contract-builder') - -describe('Linter - separate-by-one-line-in-contract', () => { - it('should raise error when items inside contract do not separated by new line', () => { - const code = contractWith(` - function a() public { - } - function b() public {} - `) - - const report = linter.processStr(code, { - rules: { 'separate-by-one-line-in-contract': 'error' } - }) - - assert.equal(report.errorCount, 1) - assertErrorMessage(report, 0, 'must be separated by one line') - }) - - it('should not raise error when items inside contract separated by new line', () => { - const code = contractWith(` - function a() public { - } - - // any comment - function b() public {} - `) - - const report = linter.processStr(code, { - rules: { 'separate-by-one-line-in-contract': 'error' } - }) - - assertNoErrors(report) - }) - - it('should not raise error when items inside contract separated by new line with comments', () => { - const code = contractWith(` - function a() public { - } - - /** - * Function b - */ - function b() public {} - `) - - const report = linter.processStr(code, { - rules: { 'separate-by-one-line-in-contract': 'error' } - }) - - assertNoErrors(report) - }) -}) diff --git a/test/rules/order/two-lines-top-level-separator.js b/test/rules/order/two-lines-top-level-separator.js deleted file mode 100644 index 1e542e35..00000000 --- a/test/rules/order/two-lines-top-level-separator.js +++ /dev/null @@ -1,38 +0,0 @@ -const assert = require('assert') -const { assertErrorMessage, assertNoErrors } = require('./../../common/asserts') -const linter = require('./../../../lib/index') - -describe('Linter - two-lines-top-level-separator', () => { - it('should raise error when contract do not surrounds with two blank lines', () => { - const code = ` - contract A {} - - contract B {} - ` - - const report = linter.processStr(code, { - rules: { 'two-lines-top-level-separator': 'error' } - }) - - assert.equal(report.errorCount, 2) - assertErrorMessage(report, 0, 'two blank') - }) - - it('should not raise error when contract do not surrounds with two blank lines', () => { - const code = ` - contract A {} - - - contract B {} - - - contract C {} - ` - - const report = linter.processStr(code, { - rules: { 'two-lines-top-level-separator': 'error' } - }) - - assertNoErrors(report) - }) -}) diff --git a/test/rules/security/compiler-fixed.js b/test/rules/security/compiler-fixed.js deleted file mode 100644 index e9d93855..00000000 --- a/test/rules/security/compiler-fixed.js +++ /dev/null @@ -1,142 +0,0 @@ -const assert = require('assert') -const linter = require('./../../../lib/index') -const { assertNoErrors, assertWarnsCount, assertErrorMessage } = require('./../../common/asserts') - -describe('Linter - compiler fixed', () => { - it('should disable fixed compiler error', () => { - const config = { - rules: { - 'compiler-fixed': false - } - } - - const report = linter.processStr('pragma solidity ^0.4.4;', config) - - assert.equal(report.errorCount, 0) - }) - - it('should change error to warn for fixed compiler issue', () => { - const config = { - rules: { - 'compiler-fixed': 'warn' - } - } - - const report = linter.processStr('pragma solidity ^0.4.4;', config) - - assert.equal(report.errorCount, 0) - assert.equal(report.warningCount, 1) - assert.ok(report.messages[0].message.includes('Compiler')) - }) - - it('should change error to warn for fixed compiler issue for array config', () => { - const config = { - rules: { - 'compiler-fixed': ['warn'] - } - } - - const report = linter.processStr('pragma solidity ^0.4.4;', config) - - assert.equal(report.errorCount, 0) - assert.equal(report.warningCount, 1) - assert.ok(report.messages[0].message.includes('Compiler')) - }) - - it('should disable fixed compiler error', () => { - const report = linter.processStr('pragma solidity ^0.4.4; // solhint-disable-line', { - rules: { 'compiler-fixed': 'error' } - }) - - assertNoErrors(report) - }) - - it('should disable fixed compiler error using multiline comment', () => { - const report = linter.processStr('pragma solidity ^0.4.4; /* solhint-disable-line */', { - rules: { 'compiler-fixed': 'error' } - }) - - assertNoErrors(report) - }) - - it('should disable only compiler version error', () => { - const report = linter.processStr( - ` - // solhint-disable compiler-gt-0_4 - pragma solidity ^0.4.4; - pragma solidity 0.3.4; // disabled error: Compiler version must be greater that 0.4 - `, - { - rules: { 'compiler-fixed': 'warn' } - } - ) - - assertWarnsCount(report, 1) - assertErrorMessage(report, 'Compiler version must be fixed') - }) - - it('should not disable fixed compiler error', () => { - const report = linter.processStr( - ` - /* solhint-disable compiler-gt-0_4 */ - pragma solidity ^0.4.4; - /* solhint-enable compiler-gt-0_4 */ - pragma solidity ^0.4.4; - `, - { - rules: { 'compiler-fixed': 'warn' } - } - ) - - assertWarnsCount(report, 2) - assertErrorMessage(report, 'fixed') - }) - - it('should disable all errors', () => { - const report = linter.processStr( - ` - /* solhint-disable */ - pragma solidity ^0.4.4; - pragma solidity 0.3.4; - `, - { - rules: { 'compiler-fixed': 'warn', 'compiler-gt-0_4': 'error' } - } - ) - assertNoErrors(report) - }) - - it('should disable then enable all errors', () => { - const report = linter.processStr( - ` - /* solhint-disable */ - pragma solidity ^0.4.4; - /* solhint-enable */ - pragma solidity ^0.4.4; - `, - { - rules: { 'compiler-fixed': 'warn', 'compiler-gt-0_4': 'error' } - } - ) - - assertWarnsCount(report, 1) - assertErrorMessage(report, 'fixed') - }) - - it('should not erase error', () => { - const report = linter.processStr('/* solhint-disable-next-line */', { - rules: { 'compiler-fixed': 'warn', 'compiler-gt-0_4': 'error' } - }) - - assertNoErrors(report) - }) - - it('should return pragma error', () => { - const report = linter.processStr('pragma solidity ^0.4.4;', { - rules: { 'compiler-fixed': 'warn' } - }) - - assertWarnsCount(report, 1) - assertErrorMessage(report, 'Compiler') - }) -}) diff --git a/test/rules/security/compiler-gt-0_4.js b/test/rules/security/compiler-gt-0_4.js deleted file mode 100644 index 45432b25..00000000 --- a/test/rules/security/compiler-gt-0_4.js +++ /dev/null @@ -1,135 +0,0 @@ -const assert = require('assert') -const { - assertNoErrors, - assertErrorCount, - assertWarnsCount, - assertErrorMessage -} = require('./../../common/asserts') -const linter = require('./../../../lib/index') - -describe('Linter - compiler-gt-0_4', () => { - it('should disable only one compiler error on next line', () => { - const report = linter.processStr( - ` - // solhint-disable-next-line - pragma solidity ^0.4.4; - pragma solidity 0.3.4; - `, - { - rules: { 'compiler-gt-0_4': 'error' } - } - ) - - assertErrorCount(report, 1) - }) - - it('should disable only one compiler error on previous line', () => { - const report = linter.processStr( - ` - pragma solidity 0.3.4; - // solhint-disable-previous-line - pragma solidity 0.3.4; - `, - { - rules: { 'compiler-gt-0_4': 'error' } - } - ) - - assertErrorCount(report, 1) - }) - - it('should disable only one compiler error on next line using multiline comment', () => { - const report = linter.processStr( - ` - /* solhint-disable-next-line */ - pragma solidity ^0.4.4; - pragma solidity 0.3.4; - `, - { - rules: { 'compiler-gt-0_4': 'error' } - } - ) - - assertErrorCount(report, 1) - }) - - it('should disable only one compiler error on previous line using multiline comment', () => { - const report = linter.processStr( - ` - pragma solidity ^0.4.4; - /* solhint-disable-previous-line */ - pragma solidity 0.3.4; - `, - { - rules: { 'compiler-gt-0_4': 'error' } - } - ) - - assertErrorCount(report, 1) - }) - - it('should disable only one compiler version error', () => { - const report = linter.processStr( - ` - /* solhint-disable compiler-gt-0_4 */ - pragma solidity 0.3.4; - /* solhint-enable compiler-gt-0_4 */ - pragma solidity 0.3.4; - `, - { - rules: { 'compiler-gt-0_4': 'error' } - } - ) - - assertErrorCount(report, 1) - assertErrorMessage(report, '0.4') - }) - - it('should disable all errors', () => { - const report = linter.processStr( - ` - /* solhint-disable */ - pragma solidity ^0.4.4; - pragma solidity 0.3.4; - `, - { - rules: { 'compiler-fixed': 'warn', 'compiler-gt-0_4': 'error' } - } - ) - assertNoErrors(report) - }) - - it('should disable then enable all errors', () => { - const report = linter.processStr( - ` - /* solhint-disable */ - pragma solidity ^0.4.4; - /* solhint-enable */ - pragma solidity ^0.4.4; - `, - { - rules: { 'compiler-fixed': 'warn', 'compiler-gt-0_4': 'error' } - } - ) - - assertWarnsCount(report, 1) - assertErrorMessage(report, 'fixed') - }) - - it('should not erase error', () => { - const report = linter.processStr('/* solhint-disable-next-line */', { - rules: { 'compiler-fixed': 'warn', 'compiler-gt-0_4': 'error' } - }) - - assertNoErrors(report) - }) - - it('should return compiler version error', () => { - const report = linter.processStr('pragma solidity 0.3.4;', { - rules: { 'compiler-gt-0_4': 'error' } - }) - - assert.equal(report.errorCount, 1) - assert.ok(report.reports[0].message.includes('0.4')) - }) -}) diff --git a/test/rules/security/compiler-version.js b/test/rules/security/compiler-version.js index 6abe289d..9d7da6ba 100644 --- a/test/rules/security/compiler-version.js +++ b/test/rules/security/compiler-version.js @@ -1,10 +1,5 @@ const assert = require('assert') -const { - assertNoErrors, - assertErrorCount, - assertWarnsCount, - assertErrorMessage -} = require('./../../common/asserts') +const { assertNoErrors, assertErrorCount, assertErrorMessage } = require('./../../common/asserts') const linter = require('./../../../lib/index') describe('Linter - compiler-version', () => { @@ -93,7 +88,7 @@ describe('Linter - compiler-version', () => { pragma solidity 0.3.4; `, { - rules: { 'compiler-fixed': 'warn', 'compiler-version': ['error', '^0.5.2'] } + rules: { 'compiler-version': ['error', '^0.5.2'] } } ) assertNoErrors(report) @@ -108,20 +103,12 @@ describe('Linter - compiler-version', () => { pragma solidity ^0.4.4; `, { - rules: { 'compiler-fixed': 'warn', 'compiler-version': ['error', '^0.5.2'] } + rules: { 'compiler-version': ['error', '^0.5.2'] } } ) - assertWarnsCount(report, 1) - assertErrorMessage(report, 'fixed') - }) - - it('should not erase error', () => { - const report = linter.processStr('/* solhint-disable-next-line */', { - rules: { 'compiler-fixed': 'warn', 'compiler-version': ['error', '^0.5.2'] } - }) - - assertNoErrors(report) + assertErrorCount(report, 1) + assertErrorMessage(report, '0.5.2') }) it('should return compiler version error', () => { @@ -157,6 +144,22 @@ describe('Linter - compiler-version', () => { assert.equal(report.errorCount, 0) }) + it('should not report compiler version error on range match', () => { + const report = linter.processStr('pragma solidity ^0.5.3;', { + rules: { 'compiler-version': ['error', '^0.5.2'] } + }) + + assert.equal(report.errorCount, 0) + }) + + it('should report compiler version error on range not matching', () => { + const report = linter.processStr('pragma solidity ^0.5.2;', { + rules: { 'compiler-version': ['error', '^0.5.3'] } + }) + + assert.equal(report.errorCount, 1) + }) + it('should report compiler version error on minor bump', () => { const report = linter.processStr('pragma solidity 0.6.0;', { rules: { 'compiler-version': ['error', '^0.5.2'] } diff --git a/test/rules/security/multiple-sends.js b/test/rules/security/multiple-sends.js index f8640604..5c2fa1e5 100644 --- a/test/rules/security/multiple-sends.js +++ b/test/rules/security/multiple-sends.js @@ -4,7 +4,7 @@ const funcWith = require('./../../common/contract-builder').funcWith const { assertErrorMessage, assertErrorCount } = require('./../../common/asserts') describe('Linter - multiple-sends', () => { - it('should return error that multiple send calls used in transation', () => { + it('should return the error that multiple send calls are being used in the same transaction', () => { const code = funcWith(` uint aRes = a.send(1); uint bRes = b.send(2); @@ -18,7 +18,7 @@ describe('Linter - multiple-sends', () => { assert.ok(report.reports[0].message.includes('multiple')) }) - it('should return error that multiple send calls used in loop', () => { + it('should return the error that multiple send calls are being used inside a loop', () => { const code = funcWith(` while (ac > b) { uint res = a.send(1); } `) diff --git a/test/rules/security/no-complex-fallback.js b/test/rules/security/no-complex-fallback.js index 3422cb27..1e4aa7dc 100644 --- a/test/rules/security/no-complex-fallback.js +++ b/test/rules/security/no-complex-fallback.js @@ -7,7 +7,6 @@ describe('Linter - no-complex-fallback', () => { const code = contractWith(`function () public payable { make1(); make2(); - make3(); }`) const report = linter.processStr(code, { diff --git a/test/rules/security/no-simple-event-func-name.js b/test/rules/security/no-simple-event-func-name.js deleted file mode 100644 index 8ad80830..00000000 --- a/test/rules/security/no-simple-event-func-name.js +++ /dev/null @@ -1,33 +0,0 @@ -const assert = require('assert') -const linter = require('./../../../lib/index') -const contractWith = require('./../../common/contract-builder').contractWith - -describe('Linter - no-simple-event-func-name', () => { - it('should return error that function and event names are similar I', () => { - const code = contractWith(` - event Name1(); - function name1() public payable { } - `) - - const report = linter.processStr(code, { - rules: { 'no-simple-event-func-name': 'warn' } - }) - - assert.equal(report.warningCount, 1) - assert.ok(report.reports[0].message.includes('Event and function names must be different')) - }) - - it('should return error that function and event names are similar II', () => { - const code = contractWith(` - function name1() public payable { } - event Name1(); - `) - - const report = linter.processStr(code, { - rules: { 'no-simple-event-func-name': 'warn' } - }) - - assert.equal(report.warningCount, 1) - assert.ok(report.reports[0].message.includes('Event and function names must be different')) - }) -})