From 8ab95338f9f13c74c24cfdf75653be66fce8ff5c Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Sat, 11 May 2024 17:51:48 -0300 Subject: [PATCH 1/3] fix exit codes and max-warnings on quiet --- CHANGELOG.md | 4 + README.md | 2 +- docker/Dockerfile | 2 +- e2e/formatters-test.js | 484 +++++++++++++++++++++++------------------ e2e/test.js | 178 ++++++++++----- package.json | 2 +- solhint.js | 91 +++++--- 7 files changed, 460 insertions(+), 303 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1909fa81..780446c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## [5.0.0] - 2024-05-11 +### BREAKING CHANGES + + ## [4.5.4] - 2024-04-10 ### Fixed - `gas-custom-errors` improved logic to ranged pragma versions [#573](https://github.com/protofire/solhint/pull/573) diff --git a/README.md b/README.md index 7cfca765..38361573 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ Options: -V, --version output the version number -f, --formatter [name] report formatter name (stylish, table, tap, unix, json, compact, sarif) - -w, --max-warnings [maxWarningsNumber] number of allowed warnings + -w, --max-warnings [maxWarningsNumber] number of allowed warnings, works in quiet mode as well -c, --config [file_name] file to use as your .solhint.json -q, --quiet report errors only - default: false --ignore-path [file_name] file to use as your .solhintignore diff --git a/docker/Dockerfile b/docker/Dockerfile index 21faaad1..5c5cb0f1 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,5 +1,5 @@ FROM node:20-alpine LABEL maintainer="diego.bale@protofire.io" -ENV VERSION=4.5.4 +ENV VERSION=5.0.0 RUN npm install -g solhint@"$VERSION" \ No newline at end of file diff --git a/e2e/formatters-test.js b/e2e/formatters-test.js index 107b7306..e313aafd 100644 --- a/e2e/formatters-test.js +++ b/e2e/formatters-test.js @@ -6,54 +6,84 @@ const path = require('path') const shell = require('shelljs') const url = require('url') -function useFixture(dir) { - beforeEach(`switch to ${dir}`, function () { - const fixturePath = path.join(__dirname, dir) - - const tmpDirContainer = os.tmpdir() - this.testDirPath = path.join(tmpDirContainer, `solhint-tests-${dir}`) - - fs.ensureDirSync(this.testDirPath) - fs.emptyDirSync(this.testDirPath) +const EXIT_CODES = { BAD_OPTIONS: 255, OK: 0, REPORTED_ERRORS: 1 } + +// ================================================================== +// use these lines and function to execute locally in TEST directory +// ================================================================== +// const E2E = false +// const PATH = './e2e/06-formatters/' +// const NODE = 'node ' +// const SUFFIX = ` -c ${PATH}.solhint.json ` + +function updateFilePath(jsonObject, prefix) { + return jsonObject.map((jsonObject) => ({ + ...jsonObject, + filePath: prefix + jsonObject.filePath, + })) +} - fs.copySync(fixturePath, this.testDirPath) +// ================================================================== +// use these two lines to E2E +// ================================================================== +const E2E = true +const PATH = '' +const NODE = '' +const SUFFIX = '' - shell.cd(this.testDirPath) - }) -} +let foo1Output +let foo2Output describe('e2e', function () { describe('formatter tests', () => { + if (E2E) { + const outputs = require('./06-formatters/helpers/helpers.js') + foo1Output = outputs.foo1Output + foo2Output = outputs.foo2Output + } else { + const outputsE2E = require('../e2e/06-formatters/helpers/helpers') + foo1Output = updateFilePath(outputsE2E.foo1Output, PATH) + foo2Output = updateFilePath(outputsE2E.foo2Output, PATH) + } // Foo contract has 1 error and 6 warnings // Foo2 contract has 3 warnings // Foo3 contract has no warnings and no errors - const { foo1Output, foo2Output } = require('./06-formatters/helpers/helpers.js') - const PATH = '' - useFixture('06-formatters') + if (E2E) useFixture('06-formatters') - it('should fail when wrong formatter is specify', () => { + it('should give an error msg when wrong formatter is specify', () => { const formatterType = 'wrongOne' - const { code } = shell.exec(`solhint ${PATH}contracts/Foo2.sol -f ${formatterType}`) - expect(code).to.equal(1) + const { code, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo2.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) + expect(stderr).to.include('There was a problem loading formatter option') }) 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 ${PATH}contracts/Foo1.sol -f ${formatterType}`) - expect(code).to.equal(0) + it('should return error when file does not exist and unix is the formatter', () => { + const { code, stdout, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo1.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) expect(stdout.trim()).to.be.empty + expect(stderr).to.include('No files to lint! check glob arguments') }) it('should return nothing when file exists and there is no error/warning', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo3.sol -f ${formatterType}`) - expect(code).to.equal(0) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.be.empty }) + it('should make the output report with unix formatter for Foo2', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo2.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo2.sol -f ${formatterType}${SUFFIX}` + ) const reportLines = stdout.split('\n') let expectedLine @@ -62,14 +92,15 @@ describe('e2e', function () { expectedLine = `${foo2Output[i].filePath}:${foo2Output[i].line}:${foo2Output[i].column}: ${foo2Output[i].message} [${foo2Output[i].severity}/${foo2Output[i].ruleId}]` expect(reportLines[i]).to.equal(expectedLine) } - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) 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 and Foo3', () => { const { code, stdout } = shell.exec( - `solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}` + `${NODE}solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` ) const reportLines = stdout.split('\n') @@ -81,7 +112,7 @@ describe('e2e', function () { expect(reportLines[i]).to.equal(expectedLine) } // because there's an error - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) const finalLine = '10 problem/s (1 error/s, 9 warning/s)' expect(reportLines[reportLines.length - 2]).to.contain(finalLine) @@ -91,18 +122,26 @@ describe('e2e', function () { 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 ${PATH}contracts/Foo1.sol -f ${formatterType}`) - expect(code).to.equal(0) + it('should return error when file does not exist and json is the formatter', () => { + const { code, stdout, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo1.sol -f ${formatterType}${SUFFIX}` + ) + + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) expect(stdout.trim()).to.be.empty + expect(stderr).to.include('No files to lint! check glob arguments') }) it('should return nothing when file exists and there is no error/warning', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo3.sol -f ${formatterType}`) - expect(code).to.equal(0) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.be.empty }) it('should make the output report with json formatter for Foo2', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo2.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo2.sol -f ${formatterType}${SUFFIX}` + ) const expectedFinalOutput = foo2Output.concat([{ conclusion: '3 problem/s (3 warning/s)' }]) @@ -111,11 +150,11 @@ describe('e2e', function () { const strOutput = JSON.stringify(objectOutput) const strExpected = JSON.stringify(expectedFinalOutput) expect(strExpected).to.equal(strOutput) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) }) it('should make the output report with json formatter for Foo and Foo2 and Foo3', () => { const { code, stdout } = shell.exec( - `solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}` + `${NODE}solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` ) const expectedFinalOutput = foo1Output @@ -128,26 +167,33 @@ describe('e2e', function () { const strExpected = JSON.stringify(expectedFinalOutput) expect(strExpected).to.equal(strOutput) // There's an error, that is why exit code is 1 - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) }) }) describe('compact formatter tests', () => { const formatterType = 'compact' - it('should return nothing when file does not exist and compact is the formatter', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo1.sol -f ${formatterType}`) - expect(code).to.equal(0) + it('should return error when file does not exist and compact is the formatter', () => { + const { code, stdout, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo1.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) expect(stdout.trim()).to.be.empty + expect(stderr).to.include('No files to lint! check glob arguments') }) it('should return nothing when file exists and there is no error/warning', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo3.sol -f ${formatterType}`) - expect(code).to.equal(0) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.be.empty }) it('should make the output report with compact formatter for Foo2', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo2.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo2.sol -f ${formatterType}${SUFFIX}` + ) const reportLines = stdout.split('\n') let expectedLine @@ -156,14 +202,14 @@ describe('e2e', function () { expectedLine = `${foo2Output[i].filePath}: line ${foo2Output[i].line}, col ${foo2Output[i].column}, ${foo2Output[i].severity} - ${foo2Output[i].message} (${foo2Output[i].ruleId})` expect(reportLines[i]).to.equal(expectedLine) } - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) const finalLine = '3 problem/s (3 warning/s)' expect(reportLines[reportLines.length - 2]).to.equal(finalLine) }) it('should make the output report with compact formatter for Foo and Foo2 and Foo3', () => { const { code, stdout } = shell.exec( - `solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}` + `${NODE}solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` ) const reportLines = stdout.split('\n') @@ -175,7 +221,7 @@ describe('e2e', function () { expect(reportLines[i]).to.equal(expectedLine) } // because there's an error - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) const finalLine = '10 problem/s (1 error/s, 9 warning/s)' expect(reportLines[reportLines.length - 2]).to.contain(finalLine) @@ -185,19 +231,26 @@ describe('e2e', function () { describe('stylish formatter tests', () => { const formatterType = 'stylish' - it('should return nothing when file does not exist and stylish is the formatter', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo1.sol -f ${formatterType}`) - expect(code).to.equal(0) + it('should return error when file does not exist and stylish is the formatter', () => { + const { code, stdout, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo1.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) expect(stdout.trim()).to.be.empty + expect(stderr).to.include('No files to lint! check glob arguments') }) it('should return nothing when file exists and there is no error/warning', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo3.sol -f ${formatterType}`) - expect(code).to.equal(0) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.be.empty }) it('should make the output report with stylish formatter for Foo2', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo2.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo2.sol -f ${formatterType}${SUFFIX}` + ) const reportLines = stdout.split('\n') let expectedLine = foo2Output[0].filePath @@ -210,14 +263,14 @@ describe('e2e', function () { expect(reportLines[i].replace(/\s/g, '')).to.equal(expectedLine.replace(/\s/g, '')) } - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) const finalLine = '\u2716 3 problems (0 errors, 3 warnings)' expect(reportLines[reportLines.length - 3]).to.equal(finalLine) }) it('should make the output report with stylish formatter for Foo and Foo2 and Foo3', () => { const { code, stdout } = shell.exec( - `solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}` + `${NODE}solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` ) const reportLines = stdout.split('\n') @@ -240,7 +293,7 @@ describe('e2e', function () { expect(reportLines[i].replace(/\s/g, '')).to.equal(expectedLine.replace(/\s/g, '')) } - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) const finalLine = '\u2716 10 problems (1 error, 9 warnings)' expect(reportLines[reportLines.length - 3]).to.equal(finalLine) @@ -250,27 +303,31 @@ describe('e2e', function () { describe('tap formatter tests', () => { const formatterType = 'tap' - it('should return TAP header when file does not exist and tap is the formatter', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo1.sol -f ${formatterType}`) - - const reportLines = stdout.split('\n') - expect(reportLines[0]).to.eq('TAP version 13') - expect(reportLines[1]).to.eq('1..0') + it('should return error when file does not exist and tap is the formatter', () => { + const { code, stdout, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo1.sol -f ${formatterType}${SUFFIX}` + ) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) + expect(stdout.trim()).to.be.empty + expect(stderr).to.include('No files to lint! check glob arguments') }) it('should return TAP header [ok 1] when file exists and there is no error/warning', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo3.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` + ) const reportLines = stdout.split('\n') expect(reportLines[0]).to.eq('TAP version 13') expect(reportLines[1]).to.eq('1..1') expect(reportLines[2]).to.eq(`ok 1 - ${PATH}contracts/Foo3.sol`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) }) it('should make the output report with tap formatter for Foo2', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo2.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo2.sol -f ${formatterType}${SUFFIX}` + ) const reportLines = stdout.split('\n') expect(reportLines[0]).to.eq('TAP version 13') @@ -283,7 +340,7 @@ describe('e2e', function () { expect(reportLines[8]).to.eq(` column: ${foo2Output[0].column}`) expect(reportLines[9]).to.eq(` ruleId: ${foo2Output[0].ruleId}`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) }) }) @@ -302,20 +359,19 @@ describe('e2e', function () { const tableHeader2 = '╟──────────┼──────────┼──────────┼────────────────────────────────────────────────────────┼──────────────────────╢' - it('should return TABLE Footer when file does not exist and table is the formatter', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo1.sol -f ${formatterType}`) - const reportLines = stdout.split('\n') - expect(reportLines[1]).to.eq(tableFooter1) - expect(reportLines[2]).to.eq(tableFooter2) - expect(reportLines[3]).to.eq(tableFooter3) - expect(reportLines[4]).to.eq(tableFooter4) - expect(reportLines[5]).to.eq(tableFooter5) - - expect(code).to.equal(0) + it('should return error when file does not exist and table is the formatter', () => { + const { code, stdout, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo1.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) + expect(stdout.trim()).to.be.empty + expect(stderr).to.include('No files to lint! check glob arguments') }) it('should return TABLE Footer when file exists and there is no error/warning', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo3.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` + ) const reportLines = stdout.split('\n') expect(reportLines[1]).to.eq(tableFooter1) expect(reportLines[2]).to.eq(tableFooter2) @@ -323,10 +379,12 @@ describe('e2e', function () { expect(reportLines[4]).to.eq(tableFooter4) expect(reportLines[5]).to.eq(tableFooter5) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) }) it('should make the output report with table formatter for Foo', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo.sol -f ${formatterType}${SUFFIX}` + ) const reportLines = stdout.split('\n') expect(reportLines[1]).to.eq(foo1Output[0].filePath) @@ -342,114 +400,103 @@ describe('e2e', function () { expect(reportLines[3]).to.eq(tableHeader1) expect(reportLines[4]).to.eq(tableHeader2) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) }) }) describe('sarif formatter tests', () => { const formatterType = 'sarif' - it('should always output with correct SARIF version and tool metadata', () => { - const { code, stdout } = shell.exec(`solhint -f ${formatterType}`) - const sarifOutput = JSON.parse(stdout) - - expect(code).to.equal(0) - expect(sarifOutput['$schema']).to.eq('http://json.schemastore.org/sarif-2.1.0-rtm.5') - expect(sarifOutput.version).to.eq('2.1.0') - expect(sarifOutput.runs[0].tool.driver.name).to.eq('solhint') - expect(sarifOutput.runs[0].tool.driver.informationUri).to.eq('https://github.com/protofire/solhint') - expect(sarifOutput.runs[0].tool.driver.version).to.eq(require('../package.json').version) - }) - - it('should output with empty results and no artifacts when file does not exist and sarif is the formatter', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo1.sol -f ${formatterType}`) - const sarifOutput = JSON.parse(stdout) - - expect(code).to.equal(0) - expect(sarifOutput.runs[0].artifacts).to.be.undefined - expect(sarifOutput.runs[0].results).to.be.empty + it('should return error when file does not exist and sarif is the formatter', () => { + const { code, stdout, stderr } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo1.sol -f ${formatterType}${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) + expect(stdout.trim()).to.be.empty + expect(stderr).to.include('No files to lint! check glob arguments') }) it('should output with sarif formatter for Foo2', () => { - const { code, stdout } = shell.exec(`solhint ${PATH}contracts/Foo2.sol -f ${formatterType}`) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PATH}contracts/Foo2.sol -f ${formatterType}${SUFFIX}` + ) const sarifOutput = JSON.parse(stdout) const expectedUriPath = url.pathToFileURL(`${PATH}contracts/Foo2.sol`).toString() const expectedResults = [ { - level: "warning", + level: 'warning', message: { - text: "Constant name must be in capitalized SNAKE_CASE" + text: 'Constant name must be in capitalized SNAKE_CASE', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPath, - index: 0 + index: 0, }, region: { startLine: 5, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "const-name-snakecase" + ruleId: 'const-name-snakecase', }, { - level: "warning", + level: 'warning', message: { - text: "Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)" + text: 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPath, - index: 0 + index: 0, }, region: { startLine: 7, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "func-visibility" + ruleId: 'func-visibility', }, { - level: "warning", + level: 'warning', message: { - text: "Code contains empty blocks" + text: 'Code contains empty blocks', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPath, - index: 0 + index: 0, }, region: { startLine: 7, - startColumn: 19 - } - } - } + startColumn: 19, + }, + }, + }, ], - ruleId: "no-empty-blocks" - } + ruleId: 'no-empty-blocks', + }, ] - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) expect(sarifOutput.runs[0].artifacts[0].location.uri).to.eq(expectedUriPath) expect(sarifOutput.runs[0].results).to.deep.equal(expectedResults) }) - it('should output with sarif formatter for Foo and Foo2 and Foo3', () => { const { code, stdout } = shell.exec( - `solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}` + `${NODE}solhint ${PATH}contracts/Foo.sol ${PATH}contracts/Foo2.sol ${PATH}contracts/Foo3.sol -f ${formatterType}${SUFFIX}` ) const sarifOutput = JSON.parse(stdout) @@ -458,225 +505,240 @@ describe('e2e', function () { const expectedUriPathFoo3 = url.pathToFileURL(`${PATH}contracts/Foo3.sol`).toString() const expectedResults = [ { - level: "error", + level: 'error', message: { - text: "Compiler version >=0.6.0 does not satisfy the ^0.8.0 semver requirement" + text: 'Compiler version >=0.6.0 does not satisfy the ^0.8.0 semver requirement', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo, - index: 0 + index: 0, }, region: { startLine: 2, - startColumn: 1 - } - } - } + startColumn: 1, + }, + }, + }, ], - ruleId: "compiler-version" + ruleId: 'compiler-version', }, { - level: "warning", + level: 'warning', message: { - text: "Constant name must be in capitalized SNAKE_CASE" + text: 'Constant name must be in capitalized SNAKE_CASE', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo, - index: 0 + index: 0, }, region: { startLine: 5, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "const-name-snakecase" + ruleId: 'const-name-snakecase', }, { - level: "warning", + level: 'warning', message: { - text: "Explicitly mark visibility of state" + text: 'Explicitly mark visibility of state', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo, - index: 0 + index: 0, }, region: { startLine: 6, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "state-visibility" + ruleId: 'state-visibility', }, { - level: "warning", + level: 'warning', message: { - text: "'TEST2' should start with _" + text: "'TEST2' should start with _", }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo, - index: 0 + index: 0, }, region: { startLine: 6, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "private-vars-leading-underscore" + ruleId: 'private-vars-leading-underscore', }, { - level: "warning", + level: 'warning', message: { - text: "Variable name must be in mixedCase" + text: 'Variable name must be in mixedCase', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo, - index: 0 + index: 0, }, region: { startLine: 6, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "var-name-mixedcase" + ruleId: 'var-name-mixedcase', }, { - level: "warning", + level: 'warning', message: { - text: "Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)" + text: 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo, - index: 0 + index: 0, }, region: { startLine: 8, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "func-visibility" + ruleId: 'func-visibility', }, { - level: "warning", + level: 'warning', message: { - text: "Code contains empty blocks" + text: 'Code contains empty blocks', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo, - index: 0 + index: 0, }, region: { startLine: 8, - startColumn: 19 - } - } - } + startColumn: 19, + }, + }, + }, ], - ruleId: "no-empty-blocks" + ruleId: 'no-empty-blocks', }, { - level: "warning", + level: 'warning', message: { - text: "Constant name must be in capitalized SNAKE_CASE" + text: 'Constant name must be in capitalized SNAKE_CASE', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo2, - index: 1 + index: 1, }, region: { startLine: 5, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "const-name-snakecase" + ruleId: 'const-name-snakecase', }, { - level: "warning", + level: 'warning', message: { - text: "Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)" + text: 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo2, - index: 1 + index: 1, }, region: { startLine: 7, - startColumn: 5 - } - } - } + startColumn: 5, + }, + }, + }, ], - ruleId: "func-visibility" + ruleId: 'func-visibility', }, { - level: "warning", + level: 'warning', message: { - text: "Code contains empty blocks" + text: 'Code contains empty blocks', }, locations: [ { physicalLocation: { artifactLocation: { uri: expectedUriPathFoo2, - index: 1 + index: 1, }, region: { startLine: 7, - startColumn: 19 - } - } - } + startColumn: 19, + }, + }, + }, ], - ruleId: "no-empty-blocks" - } + ruleId: 'no-empty-blocks', + }, ] // There's an error, that is why exit code is 1 - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) expect(sarifOutput.runs[0].artifacts[0].location.uri).to.eq(expectedUriPathFoo) expect(sarifOutput.runs[0].artifacts[1].location.uri).to.eq(expectedUriPathFoo2) expect(sarifOutput.runs[0].artifacts[2].location.uri).to.eq(expectedUriPathFoo3) expect(sarifOutput.runs[0].results).to.deep.equal(expectedResults) }) - }) }) }) + +function useFixture(dir) { + beforeEach(`switch to ${dir}`, function () { + const fixturePath = path.join(__dirname, dir) + + const tmpDirContainer = os.tmpdir() + this.testDirPath = path.join(tmpDirContainer, `solhint-tests-${dir}`) + + fs.ensureDirSync(this.testDirPath) + fs.emptyDirSync(this.testDirPath) + + fs.copySync(fixturePath, this.testDirPath) + + shell.cd(this.testDirPath) + }) +} diff --git a/e2e/test.js b/e2e/test.js index ec40127c..c9c4dc42 100644 --- a/e2e/test.js +++ b/e2e/test.js @@ -6,46 +6,67 @@ const os = require('os') const path = require('path') const shell = require('shelljs') -function useFixture(dir) { - beforeEach(`switch to ${dir}`, function () { - const fixturePath = path.join(__dirname, dir) - - const tmpDirContainer = os.tmpdir() - this.testDirPath = path.join(tmpDirContainer, `solhint-tests-${dir}`) - - fs.ensureDirSync(this.testDirPath) - fs.emptyDirSync(this.testDirPath) - - fs.copySync(fixturePath, this.testDirPath) - - shell.cd(this.testDirPath) - }) +const EXIT_CODES = { BAD_OPTIONS: 255, OK: 0, REPORTED_ERRORS: 1 } + +// ================================================================== +// use these lines to execute locally in TEST directory +// ================================================================== +// const E2E = false +// const NODE = 'node ' + +// ================================================================== +// use these lines to E2E +// ================================================================== +const E2E = true +const NODE = '' + +function prepareContext(path) { + let PREFIX + let SUFFIX + + if (E2E) { + useFixture(path) + PREFIX = '' + SUFFIX = ' --disc' + } else { + PREFIX = `./e2e/${path}/` + SUFFIX = `-c ${PREFIX}.solhint.json --disc` + } + return { PREFIX, SUFFIX } } describe('e2e', function () { + if (!E2E) shell.exec(`rm ./.solhint.json`) + describe('no config', function () { - useFixture('01-no-config') + const PATH = '01-no-config' + prepareContext(PATH) - it('should fail', function () { - const { code } = shell.exec('solhint Foo.sol') + it('should fail when config file does not exists', function () { + const { code, stderr } = shell.exec(`${NODE}solhint Foo.sol -c ./noconfig/.solhint.json`) - expect(code).to.equal(1) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) + expect(stderr).to.include('couldnt be found') }) it('should create an initial config with --init', function () { - const { code } = shell.exec('solhint --init') + let solhintConfigPath + if (E2E) solhintConfigPath = path.join(this.testDirPath, '.solhint.json') + else solhintConfigPath = './.solhint.json' - expect(code).to.equal(0) + const { code } = shell.exec(`${NODE}solhint --init`) - const solhintConfigPath = path.join(this.testDirPath, '.solhint.json') + if (E2E) solhintConfigPath = path.join(this.testDirPath, '.solhint.json') + else solhintConfigPath = './.solhint.json' + expect(code).to.equal(EXIT_CODES.OK) expect(fs.existsSync(solhintConfigPath)).to.be.true }) it('should print usage if called without arguments', function () { - const { code, stdout } = shell.exec('solhint') + const { code, stdout } = shell.exec(`${NODE}solhint`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout).to.include('Usage: solhint [options]') expect(stdout).to.include('Linter for Solidity programming language') expect(stdout).to.include('linting of source code data provided to STDIN') @@ -53,35 +74,46 @@ describe('e2e', function () { }) describe('empty-config', function () { - useFixture('02-empty-solhint-json') + const PATH = '02-empty-solhint-json' + const { PREFIX, SUFFIX } = prepareContext(PATH) it('should print nothing', function () { - const { code, stdout } = shell.exec('solhint Foo.sol') + const { code, stdout } = shell.exec(`${NODE}solhint ${PREFIX}Foo.sol ${SUFFIX}`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.equal('') }) it('should show warning when using --init', function () { - const { code, stdout } = shell.exec('solhint --init') + const { code, stdout } = shell.exec(`${NODE}solhint --init`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) expect(stdout.trim()).to.equal('Configuration file already exists') }) }) describe('no-empty-blocks', function () { - useFixture('03-no-empty-blocks') + const PATH = '03-no-empty-blocks' + const { PREFIX, SUFFIX } = prepareContext(PATH) + + describe('No contracts to lint', function () { + it('should fail with appropiate message', function () { + const { code, stderr } = shell.exec(`${NODE}solhint Foo.sol ${SUFFIX}`) + + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) + expect(stderr).to.include('No files to lint!') + }) + }) - it('should end correctly (exit w/0), found 1 error', function () { - const { code, stdout } = shell.exec('solhint Foo.sol') + it('should end with REPORTED_ERRORS = 1 because report contains errors', function () { + const { code, stdout } = shell.exec(`${NODE}solhint ${PREFIX}Foo.sol ${SUFFIX}`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) expect(stdout.trim()).to.contain('Code contains empty blocks') }) it('should work with stdin, exit 0, found 1 error', async function () { - const child = cp.exec('solhint stdin') + const child = cp.exec(`${NODE}solhint stdin ${PREFIX}Foo.sol ${SUFFIX}`) const stdoutPromise = getStream(child.stdout) @@ -95,78 +127,114 @@ describe('e2e', function () { child.stdin.end() const code = await codePromise - - expect(code).to.equal(0) - const stdout = await stdoutPromise + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) expect(stdout.trim()).to.contain('Code contains empty blocks') }) }) describe('.sol on path', function () { - useFixture('04-dotSol-on-path') + const PATH = '04-dotSol-on-path' + const { PREFIX, SUFFIX } = prepareContext(PATH) it('should handle directory names that end with .sol', function () { - const { code } = shell.exec('solhint contracts/**/*.sol') - expect(code).to.equal(0) + const { code } = shell.exec(`${NODE}solhint ${PREFIX}contracts/**/*.sol ${SUFFIX}`) + expect(code).to.equal(EXIT_CODES.OK) }) }) describe('--max-warnings parameter tests', function () { + const PATH = '05-max-warnings' + const { PREFIX, SUFFIX } = prepareContext(PATH) + // Foo contract has 6 warnings // Foo2 contract has 1 error and 14 warnings - useFixture('05-max-warnings') const warningExceededMsg = 'Solhint found more warnings than the maximum specified' const errorFound = 'Error/s found on rules! [max-warnings] param is ignored. Fixing errors enables max-warnings' it('should not display [warnings exceeded] for max 7 warnings', function () { - const { code, stdout } = shell.exec('solhint contracts/Foo.sol --max-warnings 7') - expect(code).to.equal(0) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PREFIX}contracts/Foo.sol --max-warnings 7 ${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.not.contain(warningExceededMsg) }) - it('should display [warnings exceeded] for max 3 warnings and exit with 0', function () { - const { code, stdout } = shell.exec('solhint contracts/Foo.sol --max-warnings 3') + it('should display [warnings exceeded] for max 3 warnings and exit with 1', function () { + const { code, stdout } = shell.exec( + `${NODE}solhint ${PREFIX}contracts/Foo.sol --max-warnings 3 ${SUFFIX}` + ) - expect(code).to.equal(1) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) 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( + `${NODE}solhint ${PREFIX}contracts/Foo2.sol --max-warnings 3 ${SUFFIX}` + ) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) expect(stdout.trim()).to.contain(errorFound) }) it('should return error for Compiler version rule. No message for max-warnings', function () { - const { code, stdout } = shell.exec('solhint contracts/Foo2.sol --max-warnings 27') - expect(code).to.equal(0) + const { code, stdout } = shell.exec( + `${NODE}solhint ${PREFIX}contracts/Foo2.sol --max-warnings 27 ${SUFFIX}` + ) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) expect(stdout.trim()).to.not.contain(errorFound) }) + + it('should NOT display warningns nor error but exit with 1 because max is 3 warnings', function () { + const { code } = shell.exec( + `${NODE}solhint ${PREFIX}contracts/Foo.sol --max-warnings 3 ${SUFFIX} -q` + ) + + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) + }) }) describe('Linter - foundry-test-functions with shell', () => { + const PATH = '07-foundry-test' + const { PREFIX, SUFFIX } = prepareContext(PATH) + // Foo contract has 1 warning // FooTest contract has 1 error - useFixture('07-foundry-test') it(`should raise error for empty blocks only`, () => { - const { code, stdout } = shell.exec('solhint contracts/Foo.sol') + const { code, stdout } = shell.exec(`${NODE}solhint ${PREFIX}contracts/Foo.sol ${SUFFIX}`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.contain('Code contains empty blocks') }) it(`should raise error for wrongFunctionDefinitionName() only`, () => { - const { code, stdout } = shell.exec('solhint -c test/.solhint.json test/FooTest.sol') + const SUFFIX2 = `-c ${PREFIX}test/.solhint.json --disc` + const { code, stdout } = shell.exec(`${NODE}solhint ${PREFIX}test/FooTest.sol ${SUFFIX2}`) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.OK) expect(stdout.trim()).to.contain( - 'Function wrongFunctionDefinitionName() must match Foundry test naming convention' + 'Function wrongFunctionDefinitionName() must match Foundry test naming convention ' ) }) }) }) + +function useFixture(dir) { + beforeEach(`switch to ${dir}`, function () { + const fixturePath = path.join(__dirname, dir) + + const tmpDirContainer = os.tmpdir() + this.testDirPath = path.join(tmpDirContainer, `solhint-tests-${dir}`) + + fs.ensureDirSync(this.testDirPath) + fs.emptyDirSync(this.testDirPath) + + fs.copySync(fixturePath, this.testDirPath) + + shell.cd(this.testDirPath) + }) +} diff --git a/package.json b/package.json index ad9ac44c..5f2783f0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "solhint", - "version": "4.5.4", + "version": "5.0.0", "description": "Solidity Code Linter", "main": "lib/index.js", "keywords": [ diff --git a/solhint.js b/solhint.js index 755a910c..2cdce4da 100755 --- a/solhint.js +++ b/solhint.js @@ -12,6 +12,8 @@ const applyFixes = require('./lib/apply-fixes') const ruleFixer = require('./lib/rule-fixer') const packageJson = require('./package.json') +const EXIT_CODES = { BAD_OPTIONS: 255, OK: 0, REPORTED_ERRORS: 1 } + function init() { const version = packageJson.version program.version(version) @@ -84,7 +86,7 @@ function execMainAction() { askUserToContinue((userAnswer) => { if (userAnswer !== 'y') { console.log('\nProcess terminated by user') - process.exit(0) + process.exit(EXIT_CODES.OK) } else { // User agreed, continue with the operation. continueExecution() @@ -119,11 +121,27 @@ function executeMainActionLogic() { formatterFn = getFormatter(program.opts().formatter) } catch (ex) { console.error(ex.message) - process.exit(1) + process.exit(EXIT_CODES.BAD_OPTIONS) + } + + const customConfig = program.opts().config + if (customConfig && !fs.existsSync(customConfig)) { + console.error(`Config file "${customConfig}" couldnt be found.`) + process.exit(EXIT_CODES.BAD_OPTIONS) } - const reportLists = program.args.filter(_.isString).map(processPath) - const reports = _.flatten(reportLists) + let reports + try { + const reportLists = program.args.filter(_.isString).map(processPath) + reports = _.flatten(reportLists) + } catch (e) { + console.error(e) + process.exit(EXIT_CODES.BAD_OPTIONS) + } + if (reports.length === 0) { + console.error(`No files to lint! check glob arguments "${program.args}" and ignore files.`) + process.exit(EXIT_CODES.BAD_OPTIONS) + } if (program.opts().fix) { for (const report of reports) { @@ -146,22 +164,25 @@ function executeMainActionLogic() { fs.writeFileSync(report.filePath, output) } catch (error) { console.error('An error occurred while writing the file:', error) + process.exit(EXIT_CODES.BAD_OPTIONS) } } } } - if (program.opts().quiet) { - // filter the list of reports, to set errors only. - reports.forEach((reporter) => { - reporter.reports = reporter.reports.filter((i) => i.severity === 2) - }) - } + /// REMOVED THIS TO ALLOW PROCESS OF WARNINGS IN QUIET MODE + // if (program.opts().quiet) { + // // filter the list of reports, to set errors only. + // reports.forEach((reporter) => { + // reporter.reports = reporter.reports.filter((i) => i.severity === 2) + // }) + // } printReports(reports, formatterFn) - // exitWithCode(reports) - process.exit(0) + if (reports[0].errorCount > 0) process.exit(EXIT_CODES.REPORTED_ERRORS) + + process.exit(EXIT_CODES.OK) } function processStdin(options) { @@ -176,14 +197,16 @@ function processStdin(options) { formatterFn = getFormatter(program.opts().formatter) } catch (ex) { console.error(ex.message) - process.exit(1) + process.exit(EXIT_CODES.BAD_OPTIONS) } const reports = [report] printReports(reports, formatterFn) - process.exit(0) - // exitWithCode(reports) + + if (reports[0].errorCount > 0) process.exit(EXIT_CODES.REPORTED_ERRORS) + + process.exit(EXIT_CODES.OK) } function writeSampleConfigFile() { @@ -192,14 +215,16 @@ function writeSampleConfigFile() { "extends": "solhint:default" } ` - if (!fs.existsSync(configPath)) { fs.writeFileSync(configPath, sampleConfig) console.log('Configuration file created!') } else { - console.log('Configuration file already exists') + console.error('Configuration file already exists') + process.exit(EXIT_CODES.BAD_OPTIONS) } + + process.exit(EXIT_CODES.OK) } const readIgnore = _.memoize(() => { @@ -217,6 +242,7 @@ const readIgnore = _.memoize(() => { } catch (e) { if (program.opts().ignorePath && e.code === 'ENOENT') { console.error(`\nERROR: ${ignoreFile} is not a valid path.`) + process.exit(EXIT_CODES.BAD_OPTIONS) } return [] } @@ -227,8 +253,8 @@ const readConfig = _.memoize(() => { try { config = loadConfig(program.opts().config) } catch (e) { - console.log(e.message) - process.exit(1) + console.error(e.message) + process.exit(EXIT_CODES.BAD_OPTIONS) } const configExcludeFiles = _.flatten(config.excludedFiles) @@ -259,7 +285,7 @@ function areWarningsExceeded(reports) { function printReports(reports, formatter) { const warnings = areWarningsExceeded(reports) let finalMessage = '' - let exitWithOne = false + let maxWarnsFound = false if ( program.opts().maxWarnings && reports && @@ -270,7 +296,7 @@ function printReports(reports, formatter) { finalMessage = `Solhint found more warnings than the maximum specified (maximum: ${ program.opts().maxWarnings }, found: ${warnings.warningsCount})` - exitWithOne = true + maxWarnsFound = true } else { finalMessage = 'Error/s found on rules! [max-warnings] param is ignored. Fixing errors enables max-warnings' @@ -278,13 +304,13 @@ function printReports(reports, formatter) { } const fullReport = formatter(reports) + (finalMessage || '') - console.log(fullReport) + if (!program.opts().quiet) console.log(fullReport) if (program.opts().save) { writeStringToFile(fullReport) } - - if (exitWithOne) process.exit(1) + console.log('maxWarnsFound :>> ', maxWarnsFound) + if (maxWarnsFound) process.exit(EXIT_CODES.REPORTED_ERRORS) return reports } @@ -308,7 +334,9 @@ function writeStringToFile(data) { // console.log('File written successfully:', fileName) } catch (err) { console.error('Error writing to file:', err) + process.exit(EXIT_CODES.BAD_OPTIONS) } + process.exit(EXIT_CODES.OK) } function getFormatter(formatter) { @@ -331,16 +359,16 @@ function listRules() { configPath = args[configFileIndex + 1] if (!configPath || configPath.startsWith('-')) { console.error('Error: Invalid configuration file path after -c or --config flag.') - process.exit(1) + process.exit(EXIT_CODES.BAD_OPTIONS) } } else if (args.length !== 1) { - console.log('Error!! no additional parameters after list-rules command') - process.exit(1) + console.error('Error!! no additional parameters after list-rules command') + process.exit(EXIT_CODES.BAD_OPTIONS) } if (!fs.existsSync(configPath)) { - console.log('Error!! Configuration does not exists') - process.exit(1) + console.error('Error!! Configuration does not exists') + process.exit(EXIT_CODES.BAD_OPTIONS) } else { const config = readConfig() console.log('\nConfiguration File: \n', config) @@ -363,11 +391,6 @@ function listRules() { } } -// function exitWithCode(reports) { -// const errorsCount = reports.reduce((acc, i) => acc + i.errorCount, 0) -// process.exit(errorsCount > 0 ? 1 : 0) -// } - function checkForUpdate() { // eslint-disable-next-line import/no-extraneous-dependencies return import('latest-version') From d86157f78e179e1b7bd594308e76c2e74cb4a078 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Sat, 11 May 2024 18:24:19 -0300 Subject: [PATCH 2/3] fix e2e v1 --- CHANGELOG.md | 12 ++++++++++++ e2e/test.js | 2 +- solhint.js | 1 - 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 780446c6..e6792ed4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,19 @@ ## [5.0.0] - 2024-05-11 ### BREAKING CHANGES +#### Solhint EXIT codes +Solhint changed how the exit codes are implemented: +`Exit with 0 code` When execution was ok and there were no errors when evaluating the code according to the rules
+`Exit with 1 code` When execution was ok and there are errors reported
+`Exit with 1 code` When execution was ok and max warnings is lower than the reported warnings
+`Exit with 255 code` When there's an error in the execution (bad config, writing not allowed, wrong parameter, file not found, etc)
+ +#### Solhint QUIET mode +QUIET mode (-c quiet) option now works with the warnings and may exit with 1 if there are more than defined by user + + +

## [4.5.4] - 2024-04-10 ### Fixed - `gas-custom-errors` improved logic to ranged pragma versions [#573](https://github.com/protofire/solhint/pull/573) diff --git a/e2e/test.js b/e2e/test.js index c9c4dc42..abef3aec 100644 --- a/e2e/test.js +++ b/e2e/test.js @@ -188,7 +188,7 @@ describe('e2e', function () { expect(stdout.trim()).to.not.contain(errorFound) }) - it('should NOT display warningns nor error but exit with 1 because max is 3 warnings', function () { + it('should NOT display warnings nor error but exit with 1 because max is 3 warnings', function () { const { code } = shell.exec( `${NODE}solhint ${PREFIX}contracts/Foo.sol --max-warnings 3 ${SUFFIX} -q` ) diff --git a/solhint.js b/solhint.js index 2cdce4da..9eb0b95f 100755 --- a/solhint.js +++ b/solhint.js @@ -309,7 +309,6 @@ function printReports(reports, formatter) { if (program.opts().save) { writeStringToFile(fullReport) } - console.log('maxWarnsFound :>> ', maxWarnsFound) if (maxWarnsFound) process.exit(EXIT_CODES.REPORTED_ERRORS) return reports } From 0cec1f749440623f1a74e5adfc3ee10b48931911 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Sat, 11 May 2024 18:31:36 -0300 Subject: [PATCH 3/3] fix e2e v2 --- CHANGELOG.md | 1 + e2e/autofix-test.js | 27 ++++++++++++++------------- e2e/formatters-test.js | 12 ++++++------ e2e/test.js | 17 ++++++++--------- 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6792ed4..51ae795e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Solhint changed how the exit codes are implemented: #### Solhint QUIET mode QUIET mode (-c quiet) option now works with the warnings and may exit with 1 if there are more than defined by user +Thanks to [@juanpcapurro](https://github.com/juanpcapurro) for providing the code

## [4.5.4] - 2024-04-10 diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 16fd5daa..3edaaa29 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -7,6 +7,7 @@ const shell = require('shelljs') const spawnSync = require('spawn-sync') const E2E = true +const EXIT_CODES = { BAD_OPTIONS: 255, OK: 0, REPORTED_ERRORS: 1 } let params let currentConfig @@ -103,7 +104,7 @@ describe('e2e', function () { } ) - expect(solhintProcess.status).to.equal(0) + expect(solhintProcess.status).to.equal(EXIT_CODES.REPORTED_ERRORS) expect(solhintProcess.stdout.toString().includes('5 problems (5 errors, 0 warnings)')) }) }) @@ -129,7 +130,7 @@ describe('e2e', function () { `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` ) - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) const reportLines = stdout.split('\n') const finalLine = '5 problems (5 errors, 0 warnings)' @@ -175,8 +176,8 @@ describe('e2e', function () { expect(result).to.be.true }) - it('should execute and exit with code 0 (2)', () => { - expect(code).to.equal(0) + it('should execute and exit with code 1 (2)', () => { + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (2)', () => { @@ -221,8 +222,8 @@ describe('e2e', function () { expect(result).to.be.true }) - it('should execute and exit with code 1 (3)', () => { - expect(code).to.equal(0) + it('should execute and exit with code 0 (3)', () => { + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (3)', () => { @@ -268,7 +269,7 @@ describe('e2e', function () { }) it('should execute and exit with code 1 (4)', () => { - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (4)', () => { @@ -314,7 +315,7 @@ describe('e2e', function () { }) it('should execute and exit with code 1 (5)', () => { - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (5)', () => { @@ -361,7 +362,7 @@ describe('e2e', function () { }) it('should execute and exit with code 1 (6)', () => { - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (6)', () => { @@ -401,7 +402,7 @@ describe('e2e', function () { }) it('should execute and exit with code 1 (6)', () => { - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (6)', () => { @@ -448,7 +449,7 @@ describe('e2e', function () { }) it('should execute and exit with code 1 (7)', () => { - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (7)', () => { @@ -494,7 +495,7 @@ describe('e2e', function () { }) it('should execute and exit with code 1 (8)', () => { - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (8)', () => { @@ -540,7 +541,7 @@ describe('e2e', function () { }) it('should execute and exit with code 1 (8)', () => { - expect(code).to.equal(0) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) it('should get the right report (8)', () => { diff --git a/e2e/formatters-test.js b/e2e/formatters-test.js index e313aafd..0b955587 100644 --- a/e2e/formatters-test.js +++ b/e2e/formatters-test.js @@ -112,7 +112,7 @@ describe('e2e', function () { expect(reportLines[i]).to.equal(expectedLine) } // because there's an error - expect(code).to.equal(EXIT_CODES.OK) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) const finalLine = '10 problem/s (1 error/s, 9 warning/s)' expect(reportLines[reportLines.length - 2]).to.contain(finalLine) @@ -167,7 +167,7 @@ describe('e2e', function () { const strExpected = JSON.stringify(expectedFinalOutput) expect(strExpected).to.equal(strOutput) // There's an error, that is why exit code is 1 - expect(code).to.equal(EXIT_CODES.OK) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) }) @@ -221,7 +221,7 @@ describe('e2e', function () { expect(reportLines[i]).to.equal(expectedLine) } // because there's an error - expect(code).to.equal(EXIT_CODES.OK) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) const finalLine = '10 problem/s (1 error/s, 9 warning/s)' expect(reportLines[reportLines.length - 2]).to.contain(finalLine) @@ -293,7 +293,7 @@ describe('e2e', function () { expect(reportLines[i].replace(/\s/g, '')).to.equal(expectedLine.replace(/\s/g, '')) } - expect(code).to.equal(EXIT_CODES.OK) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) const finalLine = '\u2716 10 problems (1 error, 9 warnings)' expect(reportLines[reportLines.length - 3]).to.equal(finalLine) @@ -400,7 +400,7 @@ describe('e2e', function () { expect(reportLines[3]).to.eq(tableHeader1) expect(reportLines[4]).to.eq(tableHeader2) - expect(code).to.equal(EXIT_CODES.OK) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) }) @@ -717,7 +717,7 @@ describe('e2e', function () { ] // There's an error, that is why exit code is 1 - expect(code).to.equal(EXIT_CODES.OK) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) expect(sarifOutput.runs[0].artifacts[0].location.uri).to.eq(expectedUriPathFoo) expect(sarifOutput.runs[0].artifacts[1].location.uri).to.eq(expectedUriPathFoo2) expect(sarifOutput.runs[0].artifacts[2].location.uri).to.eq(expectedUriPathFoo3) diff --git a/e2e/test.js b/e2e/test.js index abef3aec..e6c69ef0 100644 --- a/e2e/test.js +++ b/e2e/test.js @@ -85,10 +85,10 @@ describe('e2e', function () { }) it('should show warning when using --init', function () { - const { code, stdout } = shell.exec(`${NODE}solhint --init`) - + const { code, stderr } = shell.exec(`${NODE}solhint --init`) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) - expect(stdout.trim()).to.equal('Configuration file already exists') + expect(stderr).to.include('Configuration file already exists') }) }) @@ -96,14 +96,12 @@ describe('e2e', function () { const PATH = '03-no-empty-blocks' const { PREFIX, SUFFIX } = prepareContext(PATH) - describe('No contracts to lint', function () { - it('should fail with appropiate message', function () { - const { code, stderr } = shell.exec(`${NODE}solhint Foo.sol ${SUFFIX}`) - + it('No contracts to lint should fail with appropiate message', function () { + const { code, stderr } = shell.exec(`${NODE}solhint Foo1.sol ${SUFFIX}`) + expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) expect(stderr).to.include('No files to lint!') }) - }) it('should end with REPORTED_ERRORS = 1 because report contains errors', function () { const { code, stdout } = shell.exec(`${NODE}solhint ${PREFIX}Foo.sol ${SUFFIX}`) @@ -213,9 +211,10 @@ describe('e2e', function () { it(`should raise error for wrongFunctionDefinitionName() only`, () => { const SUFFIX2 = `-c ${PREFIX}test/.solhint.json --disc` + const { code, stdout } = shell.exec(`${NODE}solhint ${PREFIX}test/FooTest.sol ${SUFFIX2}`) - expect(code).to.equal(EXIT_CODES.OK) + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) expect(stdout.trim()).to.contain( 'Function wrongFunctionDefinitionName() must match Foundry test naming convention ' )