From fe1f08ab9f94d311d600796cd4bb70ec63c1266c Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Sat, 27 Aug 2022 15:21:39 +0100 Subject: [PATCH] Remove explicitly imported function from implicit imports. (#75) * Remove explicitly imported function from implicit imports. * Fix test folder name. * Remove more false positives on implicit-import --- amarna/Result.py | 4 +-- amarna/amarna.py | 2 +- .../post_process_rules/ImplicitImportRule.py | 34 ++++++++++--------- .../implicit_import_three_test.expected | 4 +++ .../implicit_import_two_test.expected | 4 +++ tests/implicit_import_three_test/proxy.cairo | 3 ++ tests/implicit_import_three_test/utils.cairo | 29 ++++++++++++++++ tests/implicit_import_two_test/proxy.cairo | 3 ++ tests/implicit_import_two_test/utils.cairo | 29 ++++++++++++++++ 9 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 tests/expected/implicit_import_three_test.expected create mode 100644 tests/expected/implicit_import_two_test.expected create mode 100644 tests/implicit_import_three_test/proxy.cairo create mode 100644 tests/implicit_import_three_test/utils.cairo create mode 100644 tests/implicit_import_two_test/proxy.cairo create mode 100644 tests/implicit_import_two_test/utils.cairo diff --git a/amarna/Result.py b/amarna/Result.py index 1fb39d7..e032373 100644 --- a/amarna/Result.py +++ b/amarna/Result.py @@ -58,7 +58,7 @@ def __eq__(self, other: object) -> bool: class ResultMultiplePositions: def __init__( self, - filenames: str, + filenames: List[str], rule_name: str, text: str, position_list: List[PositionType], @@ -143,7 +143,7 @@ def create_result_token(filename: str, rule_name: str, text: str, token: Token) def result_multiple_positions( - filenames: str, + filenames: List[str], rule_name: str, text: str, position_list: List[PositionType], diff --git a/amarna/amarna.py b/amarna/amarna.py index 326fedc..1420f4e 100644 --- a/amarna/amarna.py +++ b/amarna/amarna.py @@ -59,7 +59,7 @@ def get_all_rule_names() -> List[str]: ) @staticmethod - def print_rule_names_and_descriptions() -> List[str]: + def print_rule_names_and_descriptions() -> None: ruleset = Amarna.load_classes_in_module(local_rules_module) + Amarna.load_classes_in_module( post_process_rules_module ) diff --git a/amarna/rules/post_process_rules/ImplicitImportRule.py b/amarna/rules/post_process_rules/ImplicitImportRule.py index 8ce6842..458ac7d 100644 --- a/amarna/rules/post_process_rules/ImplicitImportRule.py +++ b/amarna/rules/post_process_rules/ImplicitImportRule.py @@ -29,24 +29,26 @@ def run_rule(self, gathered_data: Dict) -> List[ResultMultiplePositions]: for func in declared_functions: if any(decorator in self.DECORATORS for decorator in func.decorators): - files_marked = [] + explicitly_imp: List[ImportType] = [] + explicitly_import_names: List[str] = [] for imp in import_stmts: - # check if there is any import from the location - # of the external function + # gather all explicitly imported functions + # from the location of the external function if func.file_location.endswith(imp.where_imported): - # do not flag the same file repeatedly - if imp.file_location in files_marked: - continue - - files_marked.append(imp.file_location) - - result = result_multiple_positions( - [func.file_location, imp.file_location], - self.RULE_NAME, - self.RULE_TEXT, - [func.position, imp.location], - ) - results.append(result) + explicitly_import_names.append(imp.import_name) + explicitly_imp.append(imp) + + # check if there was any import and that it was not + # explicitly imported + if explicitly_imp and func.name not in explicitly_import_names: + imp = explicitly_imp[0] + result = result_multiple_positions( + [func.file_location, imp.file_location], + self.RULE_NAME, + self.RULE_TEXT, + [func.position, imp.location], + ) + results.append(result) return results diff --git a/tests/expected/implicit_import_three_test.expected b/tests/expected/implicit_import_three_test.expected new file mode 100644 index 0000000..0cc84b4 --- /dev/null +++ b/tests/expected/implicit_import_three_test.expected @@ -0,0 +1,4 @@ +[must-check-caller-address] in utils.cairo:10:10 +[must-check-caller-address] in utils.cairo:23:10 +[unused-imports] in proxy.cairo:3:19 +[unused-imports] in proxy.cairo:3:38 \ No newline at end of file diff --git a/tests/expected/implicit_import_two_test.expected b/tests/expected/implicit_import_two_test.expected new file mode 100644 index 0000000..4c89fc5 --- /dev/null +++ b/tests/expected/implicit_import_two_test.expected @@ -0,0 +1,4 @@ +[implicit-import] [This](0) function will be imported by [here](1), even though it was not explicitly imported. in utils.cairo:19:1 and proxy.cairo:3:19 +[must-check-caller-address] in utils.cairo:10:10 +[must-check-caller-address] in utils.cairo:23:10 +[unused-imports] in proxy.cairo:3:19 \ No newline at end of file diff --git a/tests/implicit_import_three_test/proxy.cairo b/tests/implicit_import_three_test/proxy.cairo new file mode 100644 index 0000000..950f70d --- /dev/null +++ b/tests/implicit_import_three_test/proxy.cairo @@ -0,0 +1,3 @@ +%lang starknet + +from utils import auth_read_storage, auth_write_storage diff --git a/tests/implicit_import_three_test/utils.cairo b/tests/implicit_import_three_test/utils.cairo new file mode 100644 index 0000000..f8a6334 --- /dev/null +++ b/tests/implicit_import_three_test/utils.cairo @@ -0,0 +1,29 @@ +%lang starknet + +from starkware.starknet.common.syscalls import storage_read, storage_write, get_caller_address + +# Helpers for auth users to interact with contract's storage +@view +func auth_read_storage{ + syscall_ptr : felt*, + }(auth_account : felt, address : felt) -> (value : felt): + let (caller) = get_caller_address() + + assert caller = auth_account + + let (value) = storage_read(address=address) + + return (value=value) +end + +@external +func auth_write_storage{ + syscall_ptr : felt*, + }(auth_account : felt, address : felt, value : felt): + let (caller) = get_caller_address() + + assert caller = auth_account + + storage_write(address=address, value=value) + return() +end diff --git a/tests/implicit_import_two_test/proxy.cairo b/tests/implicit_import_two_test/proxy.cairo new file mode 100644 index 0000000..77d04aa --- /dev/null +++ b/tests/implicit_import_two_test/proxy.cairo @@ -0,0 +1,3 @@ +%lang starknet + +from utils import auth_read_storage \ No newline at end of file diff --git a/tests/implicit_import_two_test/utils.cairo b/tests/implicit_import_two_test/utils.cairo new file mode 100644 index 0000000..f8a6334 --- /dev/null +++ b/tests/implicit_import_two_test/utils.cairo @@ -0,0 +1,29 @@ +%lang starknet + +from starkware.starknet.common.syscalls import storage_read, storage_write, get_caller_address + +# Helpers for auth users to interact with contract's storage +@view +func auth_read_storage{ + syscall_ptr : felt*, + }(auth_account : felt, address : felt) -> (value : felt): + let (caller) = get_caller_address() + + assert caller = auth_account + + let (value) = storage_read(address=address) + + return (value=value) +end + +@external +func auth_write_storage{ + syscall_ptr : felt*, + }(auth_account : felt, address : felt, value : felt): + let (caller) = get_caller_address() + + assert caller = auth_account + + storage_write(address=address, value=value) + return() +end