From 121fbe7d5dfeb91860db060a39aad01ed0688fc4 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 24 Jul 2023 12:14:36 -0300 Subject: [PATCH] added compact format and e2e tests --- README.md | 2 +- e2e/06-formatters/.solhint.json | 3 + e2e/06-formatters/contracts/Foo.sol | 11 ++ e2e/06-formatters/contracts/Foo2.sol | 10 ++ e2e/06-formatters/contracts/Foo3.sol | 11 ++ e2e/06-formatters/helpers/helpers.js | 99 +++++++++++++++ e2e/package.json | 1 + e2e/test.js | 181 ++++++++++++++++++++++++++- lib/formatters/README.md | 12 +- lib/formatters/compact.js | 53 ++++++++ lib/formatters/json.js | 23 +++- lib/formatters/unix.js | 15 ++- solhint.js | 5 +- 13 files changed, 413 insertions(+), 13 deletions(-) create mode 100644 e2e/06-formatters/.solhint.json create mode 100644 e2e/06-formatters/contracts/Foo.sol create mode 100644 e2e/06-formatters/contracts/Foo2.sol create mode 100644 e2e/06-formatters/contracts/Foo3.sol create mode 100644 e2e/06-formatters/helpers/helpers.js create mode 100644 lib/formatters/compact.js diff --git a/README.md b/README.md index 6d8e70e3..0f010f69 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ Linter for Solidity programming language Options: -V, --version output the version number - -f, --formatter [name] report formatter name (stylish, table, tap, unix, json) + -f, --formatter [name] report formatter name (stylish, table, tap, unix, json, compact) -w, --max-warnings [maxWarningsNumber] number of allowed warnings -c, --config [file_name] file to use as your .solhint.json -q, --quiet report errors only - default: false diff --git a/e2e/06-formatters/.solhint.json b/e2e/06-formatters/.solhint.json new file mode 100644 index 00000000..3b8ee84a --- /dev/null +++ b/e2e/06-formatters/.solhint.json @@ -0,0 +1,3 @@ +{ + "extends": "solhint:all" +} diff --git a/e2e/06-formatters/contracts/Foo.sol b/e2e/06-formatters/contracts/Foo.sol new file mode 100644 index 00000000..1151c60a --- /dev/null +++ b/e2e/06-formatters/contracts/Foo.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.6.0; + +contract Foo { + uint256 public constant test1 = 1; + uint256 TEST2; + + constructor() { + + } +} diff --git a/e2e/06-formatters/contracts/Foo2.sol b/e2e/06-formatters/contracts/Foo2.sol new file mode 100644 index 00000000..9fb98013 --- /dev/null +++ b/e2e/06-formatters/contracts/Foo2.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.5.8; + +contract Foo { + uint256 public constant test1 = 1; + + constructor() { + + } +} diff --git a/e2e/06-formatters/contracts/Foo3.sol b/e2e/06-formatters/contracts/Foo3.sol new file mode 100644 index 00000000..f89c6912 --- /dev/null +++ b/e2e/06-formatters/contracts/Foo3.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.5.8; + +contract Foo { + uint256 public constant TEST1 = 1; + uint256 public value; + + function _goodContract() private { + value = TEST1; + } +} diff --git a/e2e/06-formatters/helpers/helpers.js b/e2e/06-formatters/helpers/helpers.js new file mode 100644 index 00000000..b07a0831 --- /dev/null +++ b/e2e/06-formatters/helpers/helpers.js @@ -0,0 +1,99 @@ +const foo1Output = [ + { + line: 2, + column: 1, + severity: 2, + message: 'Compiler version >=0.6.0 does not satisfy the ^0.5.8 semver requirement', + ruleId: 'compiler-version', + fix: null, + filePath: 'contracts/Foo.sol', + }, + { + line: 5, + column: 5, + severity: 3, + message: 'Constant name must be in capitalized SNAKE_CASE', + ruleId: 'const-name-snakecase', + fix: null, + filePath: 'contracts/Foo.sol', + }, + { + line: 6, + column: 5, + severity: 3, + message: 'Explicitly mark visibility of state', + ruleId: 'state-visibility', + fix: null, + filePath: 'contracts/Foo.sol', + }, + { + line: 6, + column: 5, + severity: 3, + message: "'TEST2' should start with _", + ruleId: 'private-vars-leading-underscore', + fix: null, + filePath: 'contracts/Foo.sol', + }, + { + line: 6, + column: 5, + severity: 3, + message: 'Variable name must be in mixedCase', + ruleId: 'var-name-mixedcase', + fix: null, + filePath: 'contracts/Foo.sol', + }, + { + line: 8, + column: 5, + severity: 3, + message: + 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)', + ruleId: 'func-visibility', + fix: null, + filePath: 'contracts/Foo.sol', + }, + { + line: 8, + column: 19, + severity: 3, + message: 'Code contains empty blocks', + ruleId: 'no-empty-blocks', + fix: null, + filePath: 'contracts/Foo.sol', + }, +] + +const foo2Output = [ + { + line: 5, + column: 5, + severity: 3, + message: 'Constant name must be in capitalized SNAKE_CASE', + ruleId: 'const-name-snakecase', + fix: null, + filePath: 'contracts/Foo2.sol', + }, + { + line: 7, + column: 5, + severity: 3, + message: + 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)', + ruleId: 'func-visibility', + fix: null, + filePath: 'contracts/Foo2.sol', + }, + { + line: 7, + column: 19, + severity: 3, + message: 'Code contains empty blocks', + ruleId: 'no-empty-blocks', + fix: null, + filePath: 'contracts/Foo2.sol', + }, +] + +module.exports = { foo1Output, foo2Output } diff --git a/e2e/package.json b/e2e/package.json index 8222399a..346f8245 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -10,6 +10,7 @@ "license": "MIT", "devDependencies": { "chai": "^4.3.7", + "chai-spies": "^1.0.0", "fs-extra": "^11.1.0", "get-stream": "^6.0.0", "mocha": "^10.2.0", diff --git a/e2e/test.js b/e2e/test.js index a38383c4..9a7dbde8 100644 --- a/e2e/test.js +++ b/e2e/test.js @@ -1,4 +1,5 @@ -const { expect } = require('chai') +const chai = require('chai') +const { expect } = chai const cp = require('child_process') const fs = require('fs-extra') const getStream = require('get-stream') @@ -113,7 +114,7 @@ describe('e2e', function () { }) }) - describe.only('--max-warnings parameter tests', function () { + describe('--max-warnings parameter tests', function () { // Foo contract has 6 warnings // Foo2 contract has 1 error and 14 warnings useFixture('05-max-warnings') @@ -128,13 +129,13 @@ describe('e2e', function () { }) it('should display [warnings exceeded] for max 3 warnings and exit error 1', function () { - const { code, stdout, } = shell.exec('solhint contracts/Foo.sol --max-warnings 3') + const { code, stdout } = shell.exec('solhint contracts/Foo.sol --max-warnings 3') expect(code).to.equal(1) expect(stdout.trim()).to.contain(warningExceededMsg) }) it('should return error for Compiler version rule, ignoring 3 --max-warnings', function () { - const { code, stdout } = shell.exec('solhint contracts/Foo2.sol --max-warnings 3') + const { code, stdout } = shell.exec('solhint contracts/Foo2.sol --max-warnings 3') expect(code).to.equal(1) expect(stdout.trim()).to.contain(errorFound) }) @@ -145,4 +146,176 @@ describe('e2e', function () { expect(stdout.trim()).to.not.contain(errorFound) }) }) + + describe('formatters tests', function () { + // Foo contract has 1 error and 6 warnings + // Foo2 contract has 3 warnings + useFixture('06-formatters') + + it('should display COMPACT format', function () { + const { code, stdout } = shell.exec('solhint contracts/Foo.sol -f compact') + expect(code).to.equal(0) + expect(stdout.trim()).to.not.contain(warningExceededMsg) + }) + + it('should display JSON format', function () { + const { code, stdout } = shell.exec('solhint contracts/Foo.sol -f json') + expect(code).to.equal(0) + expect(stdout.trim()).to.contain(warningExceededMsg) + }) + + it('should display STYLISH format', function () { + const { code, stdout } = shell.exec('solhint contracts/Foo2.sol -f stylish') + expect(code).to.equal(0) + expect(stdout.trim()).to.contain(errorFound) + }) + + it('should display TABLE format', function () { + const { code, stdout } = shell.exec('solhint contracts/Foo2.sol -f table') + expect(code).to.equal(0) + expect(stdout.trim()).to.not.contain(errorFound) + }) + + it('should display TAP format', function () { + const { code, stdout } = shell.exec('solhint contracts/Foo2.sol -f tap') + expect(code).to.equal(0) + expect(stdout.trim()).to.not.contain(errorFound) + }) + + it('should display UNIX format', function () { + const { code, stdout } = shell.exec('solhint contracts/Foo2.sol -f unix') + expect(code).to.equal(0) + expect(stdout.trim()).to.not.contain(errorFound) + }) + }) + + describe.only('formatter tests', () => { + const { foo1Output, foo2Output } = require('./06-formatters/helpers/helpers.js') + + // Foo contract has 1 error and 6 warnings + // Foo2 contract has 3 warnings + useFixture('06-formatters') + + it('should fail when wrong formatter is specify', () => { + const formatterType = 'wrongOne' + const { code } = shell.exec(`solhint contracts/Foo2.sol -f ${formatterType}`) + expect(code).to.equal(1) + }) + + describe('unix formatter tests', () => { + const formatterType = 'unix' + + it('should return nothing when file does not exist and unix is the formatter', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo1.sol -f ${formatterType}`) + expect(code).to.equal(0) + expect(stdout.trim()).to.be.empty + }) + it('should return nothing when file exists and there is no error/warning', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo3.sol -f ${formatterType}`) + expect(code).to.equal(0) + expect(stdout.trim()).to.be.empty + }) + it('should make the output report with unix formatter for Foo2', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo2.sol -f ${formatterType}`) + + const reportLines = stdout.trim().split('\n') + let expectedLine + let msgType + + for (let i = 0; i < reportLines.length - 3; i++) { + msgType = foo2Output[i].severity === 2 ? 'Error' : 'Warning' + expectedLine = `${foo2Output[i].filePath}:${foo2Output[i].line}:${foo2Output[i].column}: ${foo2Output[i].message} [${msgType}/${foo2Output[i].ruleId}]` + expect(reportLines[i]).to.equal(expectedLine) + } + expect(code).to.equal(0) + + const finalLine = '3 problem/s (3 warning/s) ' + expect(reportLines[reportLines.length - 2]).to.equal(finalLine) + }) + it('should make the output report with unix formatter for Foo and Foo2', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo.sol contracts/Foo2.sol -f ${formatterType}`) + + const reportLines = stdout.trim().split('\n') + + console.log('reportLines :>> ', reportLines); + console.log('reportLines length :>> ', reportLines.length); + + const joinedFoo = foo1Output.concat(foo2Output) + console.log('-----------------------------------------------------------'); + console.log('joinedFoo :>> ', joinedFoo) + console.log('==========================================================='); + let expectedLine + let msgType + + for (let i = 0; i < reportLines.length - 3; i++) { + console.log('i :>> ', i) + console.log('joinedFoo[i] :>> ', joinedFoo[i]) + msgType = joinedFoo[i].severity === 2 ? 'Error' : 'Warning' + expectedLine = `${joinedFoo[i].filePath}:${joinedFoo[i].line}:${joinedFoo[i].column}: ${joinedFoo[i].message} [${msgType}/${joinedFoo[i].ruleId}]` + expect(reportLines[i]).to.equal(expectedLine) + } + // because there's an error + expect(code).to.equal(1) + + const finalLine = '10 problem/s (1 error/s, 9 warning/s)' + expect(reportLines[reportLines.length - 2]).to.contain(finalLine) + }) + }) + describe('json formatter tests', () => { + const formatterType = 'json' + + it('should return nothing when file does not exist and json is the formatter', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo1.sol -f ${formatterType}`) + expect(code).to.equal(0) + expect(stdout.trim()).to.be.empty + }) + it('should return nothing when file exists and there is no error/warning', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo3.sol -f ${formatterType}`) + expect(code).to.equal(0) + expect(stdout.trim()).to.be.empty + }) + it('should make the output report with json formatter', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo2.sol -f ${formatterType}`) + + const reportLines = stdout.trim().split('\n') + console.log('reportLines :>> ', reportLines); + console.log('reportLines length :>> ', reportLines.length); + + // let expectedLine + // let msgType + + + // for (let i = 0; i < reportLines.length - 3; i++) { + // msgType = foo2Output[i].severity === 2 ? 'Error' : 'Warning' + // expectedLine = `${foo2Output[i].filePath}:${foo2Output[i].line}:${foo2Output[i].column}: ${foo2Output[i].message} [${msgType}/${foo2Output[i].ruleId}]` + // expect(reportLines[i]).to.equal(expectedLine) + // } + expect(code).to.equal(0) + + // const finalLine = '3 problem/s (3 warning/s) ' + // expect(reportLines[reportLines.length - 2]).to.equal(finalLine) + }) + it('should make the output report with json formatter', () => { + const { code, stdout } = shell.exec(`solhint contracts/Foo.sol contracts/Foo2.sol -f ${formatterType}`) + + const reportLines = stdout.trim().split('\n') + console.log('reportLines :>> ', reportLines); + console.log('reportLines length :>> ', reportLines.length); + // const joinedFoo = foo1Output.concat(foo2Output) + // let expectedLine + // let msgType + + + // for (let i = 0; i < reportLines.length - 3; i++) { + // msgType = foo2Output[i].severity === 2 ? 'Error' : 'Warning' + // expectedLine = `${foo2Output[i].filePath}:${foo2Output[i].line}:${foo2Output[i].column}: ${foo2Output[i].message} [${msgType}/${foo2Output[i].ruleId}]` + // expect(reportLines[i]).to.equal(expectedLine) + // } + expect(code).to.equal(1) + + // const finalLine = '3 problem/s (3 warning/s) ' + // expect(reportLines[reportLines.length - 2]).to.equal(finalLine) + }) + }) + }) }) diff --git a/lib/formatters/README.md b/lib/formatters/README.md index 5c0b4152..e56bb98f 100644 --- a/lib/formatters/README.md +++ b/lib/formatters/README.md @@ -2,8 +2,10 @@ files in this directory are pulled from eslint repository: -- table.js: eslint v5.6.0 Gajus Kuizinas -- unix.js: eslint v8.32.0 oshi-shinobu -- tap.js: eslint v8.32.0 Jonathan Kingston -- stylish.js: eslint v8.32.0 by Sindre Sorhus -- json.js: eslint v8.32.0 by Artur Lukianov & Diego Bale +- table.js: eslint - Gajus Kuizinas +- unix.js: eslint - oshi-shinobu +- tap.js: eslint - Jonathan Kingston +- stylish.js: eslint - Sindre Sorhus +- json.js: eslint - Artur Lukianov & Diego Bale +- compact.js: eslint - Nicholas C. Zakas + \ No newline at end of file diff --git a/lib/formatters/compact.js b/lib/formatters/compact.js new file mode 100644 index 00000000..95e31ae9 --- /dev/null +++ b/lib/formatters/compact.js @@ -0,0 +1,53 @@ +/** + * @fileoverview Compact reporter + * @author Nicholas C. Zakas + */ + +//------------------------------------------------------------------------------ +// Helper Functions +//------------------------------------------------------------------------------ + +/** + * Returns the severity of warning or error + * @param {Object} message message object to examine + * @returns {string} severity level + * @private + */ +function getMessageType(message) { + if (message.fatal || message.severity === 2) { + return 'Error' + } + return 'Warning' +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +// eslint-disable-next-line func-names +module.exports = function (results) { + let output = '' + let total = 0 + + results.forEach((result) => { + const messages = result.messages + + total += messages.length + + messages.forEach((message) => { + output += `${result.filePath}: ` + output += `line ${message.line || 0}` + output += `, col ${message.column || 0}` + output += `, ${getMessageType(message)}` + output += ` - ${message.message}` + output += message.ruleId ? ` (${message.ruleId})` : '' + output += '\n' + }) + }) + + if (total > 0) { + output += `\n${total} problem${total !== 1 ? 's' : ''}` + } + + return output +} diff --git a/lib/formatters/json.js b/lib/formatters/json.js index d46c7ed8..48b1b752 100644 --- a/lib/formatters/json.js +++ b/lib/formatters/json.js @@ -27,6 +27,8 @@ function getMessageType(message) { // eslint-disable-next-line func-names module.exports = function (results) { const allMessages = [] + let errors = 0 + let warnings = 0 results.forEach((result) => { const messages = result.messages @@ -34,9 +36,28 @@ module.exports = function (results) { messages.forEach((message) => { const fullObject = { ...message, filePath: result.filePath } fullObject.severity = getMessageType(fullObject) + if (fullObject.severity === 'Error') errors += 1 + else warnings += 1 allMessages.push(fullObject) }) }) - return JSON.parse(JSON.stringify(allMessages)) + let finalMessage + if (errors > 0 && warnings > 0) { + finalMessage = { + conclusion: `${errors + warnings} problem/s (${errors} error/s, ${warnings} warning/s)`, + } + } else if (errors > 0 && warnings === 0) { + finalMessage = { + conclusion: `${errors} problem/s (${errors} error/s)`, + } + } else if (errors === 0 && warnings > 0) { + finalMessage = { + conclusion: `${warnings} problem/s (${warnings} warning/s)`, + } + } + + if (finalMessage) allMessages.push(finalMessage) + + return allMessages.length > 0 ? JSON.parse(JSON.stringify(allMessages)) : '' } diff --git a/lib/formatters/unix.js b/lib/formatters/unix.js index 0029088e..581f7588 100644 --- a/lib/formatters/unix.js +++ b/lib/formatters/unix.js @@ -27,6 +27,8 @@ function getMessageType(message) { module.exports = function (results) { let output = '' let total = 0 + let errors = 0 + let warnings = 0 results.forEach((result) => { const messages = result.messages @@ -40,11 +42,22 @@ module.exports = function (results) { output += ` ${message.message} ` output += `[${getMessageType(message)}${message.ruleId ? `/${message.ruleId}` : ''}]` output += '\n' + if (message.severity === 2) errors += 1 + else warnings += 1 }) }) + let finalMessage = '' + if (errors > 0 && warnings > 0) { + finalMessage = `${errors + warnings} problem/s (${errors} error/s, ${warnings} warning/s)` + } else if (errors > 0 && warnings === 0) { + finalMessage = `${errors} problem/s (${errors} error/s)` + } else if (errors === 0 && warnings > 0) { + finalMessage = `${warnings} problem/s (${warnings} warning/s)` + } + if (total > 0) { - output += `\n${total} problem${total !== 1 ? 's' : ''}` + output += `\n${finalMessage}` } return output diff --git a/solhint.js b/solhint.js index 6d6cf188..f1d64006 100644 --- a/solhint.js +++ b/solhint.js @@ -19,7 +19,10 @@ function init() { program .name('solhint') .usage('[options] [...other_files]') - .option('-f, --formatter [name]', 'report formatter name (stylish, table, tap, unix, json)') + .option( + '-f, --formatter [name]', + 'report formatter name (stylish, table, tap, unix, json, compact)' + ) .option('-w, --max-warnings [maxWarningsNumber]', 'number of allowed warnings') .option('-c, --config [file_name]', 'file to use as your .solhint.json') .option('-q, --quiet', 'report errors only - default: false')