From 0197093a4c082dd7323543e51fbe2136968e07fd Mon Sep 17 00:00:00 2001 From: Josselin Date: Sun, 21 Oct 2018 19:17:43 +0100 Subject: [PATCH 1/3] Add exceptions to the naming convention rules --- .../detectors/naming_convention/naming_convention.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index 91acac4d2..74bf8acb6 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -6,6 +6,10 @@ class NamingConvention(AbstractDetector): """ Check if naming conventions are followed https://solidity.readthedocs.io/en/v0.4.25/style-guide.html?highlight=naming_convention%20convention#naming_convention-conventions + + Exceptions: + - Allow constant variables name/symbol/decimals to be lowercase (ERC20) + - Allow '_' at the beggining of the mixed_case match, to represent private variable and unused parameters """ ARGUMENT = 'naming-convention' @@ -19,7 +23,9 @@ class NamingConvention(AbstractDetector): @staticmethod def is_mixed_case(name): - return re.search('^[a-z]([A-Za-z0-9]+)?_?$', name) is not None + # Allow _ at the beginning to represent private variable + # or unused parameters + return re.search('^[a-z_]([A-Za-z0-9]+)?_?$', name) is not None @staticmethod def is_upper_case_with_underscores(name): @@ -108,6 +114,10 @@ class NamingConvention(AbstractDetector): 'sourceMapping': var.source_mapping}) if var.is_constant is True: + # For ERC20 compatibility + if var.name in ['symbol', 'name', 'decimals']: + continue + if self.is_upper_case_with_underscores(var.name) is False: info = "Constant '{}' is not in UPPER_CASE_WITH_UNDERSCORES, Contract: '{}' " \ .format(var.name, contract.name) From 21ff89870cd88f4947828b453d2d2ca3946061cc Mon Sep 17 00:00:00 2001 From: Josselin Date: Thu, 25 Oct 2018 13:59:47 +0100 Subject: [PATCH 2/3] NamingConvention: Check if parameters are used and if state variables are private --- scripts/travis_test.sh | 2 +- .../naming_convention/naming_convention.py | 19 +++++++++++++++---- tests/naming_convention.sol | 8 ++++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh index 2539836d8..7a6c3e5db 100755 --- a/scripts/travis_test.sh +++ b/scripts/travis_test.sh @@ -65,7 +65,7 @@ if [ $? -ne 2 ]; then fi slither tests/naming_convention.sol --disable-solc-warnings --detect-naming-convention -if [ $? -ne 10 ]; then +if [ $? -ne 12 ]; then exit 1 fi diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index 74bf8acb6..5cda7916f 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -9,7 +9,7 @@ class NamingConvention(AbstractDetector): Exceptions: - Allow constant variables name/symbol/decimals to be lowercase (ERC20) - - Allow '_' at the beggining of the mixed_case match, to represent private variable and unused parameters + - Allow '_' at the beggining of the mixed_case match for private variables and unused parameters """ ARGUMENT = 'naming-convention' @@ -23,6 +23,10 @@ class NamingConvention(AbstractDetector): @staticmethod def is_mixed_case(name): + return re.search('^[a-z]([A-Za-z0-9]+)?_?$', name) is not None + + @staticmethod + def is_mixed_case_with_underscore(name): # Allow _ at the beginning to represent private variable # or unused parameters return re.search('^[a-z_]([A-Za-z0-9]+)?_?$', name) is not None @@ -86,8 +90,11 @@ class NamingConvention(AbstractDetector): 'sourceMapping': func.source_mapping}) for argument in func.parameters: - - if self.is_mixed_case(argument.name) is False: + if argument in func.variables_read_or_written: + incorrect_naming = self.is_mixed_case(argument.name) is False + else: + incorrect_naming = self.is_mixed_case_with_underscore(argument.name) is False + if incorrect_naming: info = "Parameter '{}' is not in mixedCase, Contract: '{}', Function: '{}'' " \ .format(argument.name, argument.name, contract.name) self.log(info) @@ -129,7 +136,11 @@ class NamingConvention(AbstractDetector): 'constant': var.name, 'sourceMapping': var.source_mapping}) else: - if self.is_mixed_case(var.name) is False: + if var.visibility == 'private': + incorrect_naming = self.is_mixed_case_with_underscore(var.name) is False + else: + incorrect_naming = self.is_mixed_case(var.name) is False + if incorrect_naming: info = "Variable '{}' is not in mixedCase, Contract: '{}' ".format(var.name, contract.name) self.log(info) diff --git a/tests/naming_convention.sol b/tests/naming_convention.sol index 769739052..48c8b3e44 100644 --- a/tests/naming_convention.sol +++ b/tests/naming_convention.sol @@ -52,6 +52,14 @@ contract Test { } contract T { + uint private _myPrivateVar; + uint _myPublicVar; + + + function test(uint _unused, uint _used){ + _used;} + + uint k = 1; uint constant M = 1; From f28f3704c4c505c3f904899d8a3f0ca3939b390e Mon Sep 17 00:00:00 2001 From: Josselin Date: Fri, 26 Oct 2018 16:52:25 +0100 Subject: [PATCH 3/3] NamingConvention: Clean code --- .../naming_convention/naming_convention.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index 02aafd4c8..f3312c1f0 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -44,7 +44,7 @@ class NamingConvention(AbstractDetector): results = [] for contract in self.contracts: - if self.is_cap_words(contract.name) is False: + if not self.is_cap_words(contract.name): info = "Contract '{}' is not in CapWords".format(contract.name) self.log(info) @@ -57,7 +57,7 @@ class NamingConvention(AbstractDetector): if struct.contract != contract: continue - if self.is_cap_words(struct.name) is False: + if not self.is_cap_words(struct.name): info = "Struct '{}' is not in CapWords, Contract: '{}' ".format(struct.name, contract.name) self.log(info) @@ -71,7 +71,7 @@ class NamingConvention(AbstractDetector): if event.contract != contract: continue - if self.is_cap_words(event.name) is False: + if not self.is_cap_words(event.name): info = "Event '{}' is not in CapWords, Contract: '{}' ".format(event.name, contract.name) self.log(info) @@ -85,7 +85,7 @@ class NamingConvention(AbstractDetector): if func.contract != contract: continue - if self.is_mixed_case(func.name) is False: + if not self.is_mixed_case(func.name): info = "Function '{}' is not in mixedCase, Contract: '{}' ".format(func.name, contract.name) self.log(info) @@ -97,10 +97,10 @@ class NamingConvention(AbstractDetector): for argument in func.parameters: if argument in func.variables_read_or_written: - incorrect_naming = self.is_mixed_case(argument.name) is False + correct_naming = self.is_mixed_case(argument.name) else: - incorrect_naming = self.is_mixed_case_with_underscore(argument.name) is False - if incorrect_naming: + correct_naming = self.is_mixed_case_with_underscore(argument.name) + if not correct_naming: info = "Parameter '{}' is not in mixedCase, Contract: '{}', Function: '{}'' " \ .format(argument.name, argument.name, contract.name) self.log(info) @@ -117,7 +117,7 @@ class NamingConvention(AbstractDetector): continue if self.should_avoid_name(var.name): - if self.is_upper_case_with_underscores(var.name) is False: + if not self.is_upper_case_with_underscores(var.name): info = "Variable '{}' l, O, I should not be used, Contract: '{}' " \ .format(var.name, contract.name) self.log(info) @@ -133,7 +133,7 @@ class NamingConvention(AbstractDetector): if var.name in ['symbol', 'name', 'decimals']: continue - if self.is_upper_case_with_underscores(var.name) is False: + if not self.is_upper_case_with_underscores(var.name): info = "Constant '{}' is not in UPPER_CASE_WITH_UNDERSCORES, Contract: '{}' " \ .format(var.name, contract.name) self.log(info) @@ -145,10 +145,10 @@ class NamingConvention(AbstractDetector): 'sourceMapping': var.source_mapping}) else: if var.visibility == 'private': - incorrect_naming = self.is_mixed_case_with_underscore(var.name) is False + correct_naming = self.is_mixed_case_with_underscore(var.name) else: - incorrect_naming = self.is_mixed_case(var.name) is False - if incorrect_naming: + correct_naming = self.is_mixed_case(var.name) + if not correct_naming: info = "Variable '{}' is not in mixedCase, Contract: '{}' ".format(var.name, contract.name) self.log(info) @@ -162,7 +162,7 @@ class NamingConvention(AbstractDetector): if enum.contract != contract: continue - if self.is_cap_words(enum.name) is False: + if not self.is_cap_words(enum.name): info = "Enum '{}' is not in CapWords, Contract: '{}' ".format(enum.name, contract.name) self.log(info) @@ -176,7 +176,7 @@ class NamingConvention(AbstractDetector): if modifier.contract != contract: continue - if self.is_mixed_case(modifier.name) is False: + if not self.is_mixed_case(modifier.name): info = "Modifier '{}' is not in mixedCase, Contract: '{}' ".format(modifier.name, contract.name) self.log(info)