From 250996195d8125feb164640529bc0204b6718592 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 5 Dec 2023 12:20:40 -0300 Subject: [PATCH 1/4] fix: private-vars-leading on libraries --- .../naming/private-vars-leading-underscore.js | 57 ++++++++++--------- .../naming/private-vars-leading-underscore.js | 7 --- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/lib/rules/naming/private-vars-leading-underscore.js b/lib/rules/naming/private-vars-leading-underscore.js index bcbfdff3..f172cab9 100644 --- a/lib/rules/naming/private-vars-leading-underscore.js +++ b/lib/rules/naming/private-vars-leading-underscore.js @@ -67,7 +67,7 @@ const meta = { }, notes: [ { - note: 'This rule considers functions and variables in Libraries as well', + note: 'This rule DO NOT considers functions and variables in Libraries', }, { note: 'This rule skips external and public functions', @@ -103,26 +103,27 @@ class PrivateVarsLeadingUnderscoreChecker extends BaseChecker { this.isStrict = config && config.getObjectPropertyBoolean(ruleId, 'strict', DEFAULT_STRICTNESS) } - // ContractDefinition(node) { - // if (node.kind === 'library') { - // this.inLibrary = true - // } - // } + ContractDefinition(node) { + if (node.kind === 'library') { + this.inLibrary = true + } + } - // 'ContractDefinition:exit'() { - // this.inLibrary = false - // } + 'ContractDefinition:exit'() { + this.inLibrary = false + } FunctionDefinition(node) { - if (!node.name) { - return - } + if (!this.inLibrary) { + if (!node.name) { + return + } - const isPrivate = node.visibility === 'private' - const isInternal = node.visibility === 'internal' || node.visibility === 'default' - // const shouldHaveLeadingUnderscore = isPrivate || (!this.inLibrary && isInternal) - const shouldHaveLeadingUnderscore = isPrivate || isInternal - this.validateName(node, shouldHaveLeadingUnderscore, 'function') + const isPrivate = node.visibility === 'private' + const isInternal = node.visibility === 'internal' || node.visibility === 'default' + const shouldHaveLeadingUnderscore = isPrivate || isInternal + this.validateName(node, shouldHaveLeadingUnderscore, 'function') + } } StateVariableDeclaration() { @@ -134,18 +135,20 @@ class PrivateVarsLeadingUnderscoreChecker extends BaseChecker { } VariableDeclaration(node) { - if (!this.inStateVariableDeclaration) { - // if strict is enabled, non-state vars should not start with leading underscore - if (this.isStrict) { - this.validateName(node, false, 'variable') + if (!this.inLibrary) { + if (!this.inStateVariableDeclaration) { + // if strict is enabled, non-state vars should not start with leading underscore + if (this.isStrict) { + this.validateName(node, false, 'variable') + } + return } - return - } - const isPrivate = node.visibility === 'private' - const isInternal = node.visibility === 'internal' || node.visibility === 'default' - const shouldHaveLeadingUnderscore = isPrivate || isInternal - this.validateName(node, shouldHaveLeadingUnderscore, 'variable') + const isPrivate = node.visibility === 'private' + const isInternal = node.visibility === 'internal' || node.visibility === 'default' + const shouldHaveLeadingUnderscore = isPrivate || isInternal + this.validateName(node, shouldHaveLeadingUnderscore, 'variable') + } } validateName(node, shouldHaveLeadingUnderscore, type) { diff --git a/test/rules/naming/private-vars-leading-underscore.js b/test/rules/naming/private-vars-leading-underscore.js index dbf02a20..f9a71ba7 100644 --- a/test/rules/naming/private-vars-leading-underscore.js +++ b/test/rules/naming/private-vars-leading-underscore.js @@ -11,25 +11,18 @@ describe('Linter - private-vars-leading-underscore', () => { contractWith('function foo() {}'), contractWith('function foo() private {}'), contractWith('function foo() internal {}'), - libraryWith('function foo() {}'), - libraryWith('function foo() private {}'), - libraryWith('function foo() internal {}'), // warn when public/external names start with _ contractWith('uint public _foo;'), contractWith('uint external _foo;'), contractWith('function _foo() public {}'), contractWith('function _foo() external {}'), - libraryWith('function _foo() public {}'), - libraryWith('function _foo() external {}'), ] const SHOULD_WARN_STRICT_CASES = [ contractWith('function _foo() internal { uint _bar; }'), contractWith('function foo(uint _bar) external {}'), contractWith('function foo() public returns (uint256 _bar) {}'), - libraryWith('function _foo() returns (uint256 _bar) {}'), - libraryWith('function _foo(uint _bar) private {}'), ] const SHOULD_NOT_WARN_CASES = [ From 81e83f7ae46720ca237cfd34ec7bcdecff0ed441 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 5 Dec 2023 12:21:09 -0300 Subject: [PATCH 2/4] fix: private-vars-leading on libraries --- docs/rules/naming/private-vars-leading-underscore.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/naming/private-vars-leading-underscore.md b/docs/rules/naming/private-vars-leading-underscore.md index bba9e897..d3dea4ba 100644 --- a/docs/rules/naming/private-vars-leading-underscore.md +++ b/docs/rules/naming/private-vars-leading-underscore.md @@ -30,7 +30,7 @@ This rule accepts an array of options: ``` ### Notes -- This rule considers functions and variables in Libraries as well +- This rule DO NOT considers functions and variables in Libraries - This rule skips external and public functions - This rule skips external and public state variables - See [here](https://docs.soliditylang.org/en/latest/style-guide.html#underscore-prefix-for-non-external-functions-and-variables) for further information From b5631939b4425531347b2756794ebbab4db7ec14 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 5 Dec 2023 12:32:26 -0300 Subject: [PATCH 3/4] fix: private-vars-leading on libraries --- .../private-vars-underscore/Foo1AfterFix.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/e2e/08-autofix/private-vars-underscore/Foo1AfterFix.sol b/e2e/08-autofix/private-vars-underscore/Foo1AfterFix.sol index 87b80ad6..42c05d95 100644 --- a/e2e/08-autofix/private-vars-underscore/Foo1AfterFix.sol +++ b/e2e/08-autofix/private-vars-underscore/Foo1AfterFix.sol @@ -1,21 +1,21 @@ pragma solidity 0.8.0; library libraryName { - uint256 internal _lzarasa1; - uint256 internal _lzarasa2 = 2; + uint256 internal lzarasa1; + uint256 internal lzarasa2 = 2; uint256 internal _lzarasa3; - uint256 private _lzarasa4; - uint256 private _lzarasa5 = 5; + uint256 private lzarasa4; + uint256 private lzarasa5 = 5; uint256 private _lzarasa6; - uint256 public lzarasa7; - uint256 public lzarasa8 = 8; + uint256 public _lzarasa7; + uint256 public _lzarasa8 = 8; uint256 public lzarasa9; function fofo1() public {} - function fofo2() public {} + function _fofo2() public {} function _fofo3() internal {} - function _fofo4() internal {} + function fofo4() internal {} } contract Foo1 { From 5313f07040e2f17fcfd292a9159ebfe2c08eba17 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 5 Dec 2023 12:49:38 -0300 Subject: [PATCH 4/4] fix: private-vars-leading on libraries --- e2e/autofix-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 1f11d184..5579f83e 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -257,7 +257,7 @@ describe('e2e', function () { expect(code).to.equal(1) const reportLines = stdout.split('\n') - const finalLine = '27 problems (27 errors, 0 warnings)' + const finalLine = '19 problems (19 errors, 0 warnings)' expect(reportLines[reportLines.length - 3]).to.contain(finalLine) result = compareTextFiles(currentFile, afterFixFile)