From b45753c32b565893473b28ef5bc2c4a2f056aa8e Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Tue, 23 Aug 2022 11:56:58 +0100 Subject: [PATCH 1/3] Remove explicitely imported function from implicit imports. --- amarna/Result.py | 4 +-- amarna/amarna.py | 2 +- .../post_process_rules/ImplicitImportRule.py | 4 +++ tests/implicit_import_test_two/proxy.cairo | 3 ++ tests/implicit_import_test_two/utils.cairo | 29 +++++++++++++++++++ 5 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 tests/implicit_import_test_two/proxy.cairo create mode 100644 tests/implicit_import_test_two/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..cb2a138 100644 --- a/amarna/rules/post_process_rules/ImplicitImportRule.py +++ b/amarna/rules/post_process_rules/ImplicitImportRule.py @@ -39,6 +39,10 @@ def run_rule(self, gathered_data: Dict) -> List[ResultMultiplePositions]: if imp.file_location in files_marked: continue + # ignore explicitely imported functions + if imp.import_name == func.name: + continue + files_marked.append(imp.file_location) result = result_multiple_positions( diff --git a/tests/implicit_import_test_two/proxy.cairo b/tests/implicit_import_test_two/proxy.cairo new file mode 100644 index 0000000..77d04aa --- /dev/null +++ b/tests/implicit_import_test_two/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_test_two/utils.cairo b/tests/implicit_import_test_two/utils.cairo new file mode 100644 index 0000000..f8a6334 --- /dev/null +++ b/tests/implicit_import_test_two/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 From 6eb865dadda05c90c289a8078edc6352c8e84040 Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Tue, 23 Aug 2022 14:35:23 +0100 Subject: [PATCH 2/3] Fix test folder name. --- tests/expected/implicit_import_two_test.expected | 4 ++++ .../proxy.cairo | 0 .../utils.cairo | 0 3 files changed, 4 insertions(+) create mode 100644 tests/expected/implicit_import_two_test.expected rename tests/{implicit_import_test_two => implicit_import_two_test}/proxy.cairo (100%) rename tests/{implicit_import_test_two => implicit_import_two_test}/utils.cairo (100%) 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_test_two/proxy.cairo b/tests/implicit_import_two_test/proxy.cairo similarity index 100% rename from tests/implicit_import_test_two/proxy.cairo rename to tests/implicit_import_two_test/proxy.cairo diff --git a/tests/implicit_import_test_two/utils.cairo b/tests/implicit_import_two_test/utils.cairo similarity index 100% rename from tests/implicit_import_test_two/utils.cairo rename to tests/implicit_import_two_test/utils.cairo From 7a06dde14ed2d6400dd2d4bd7320d4e6fcbd630d Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Fri, 26 Aug 2022 15:42:24 +0100 Subject: [PATCH 3/3] Remove more false positives on implicit-import --- .../post_process_rules/ImplicitImportRule.py | 38 +++++++++---------- .../implicit_import_three_test.expected | 4 ++ tests/implicit_import_three_test/proxy.cairo | 3 ++ tests/implicit_import_three_test/utils.cairo | 29 ++++++++++++++ 4 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 tests/expected/implicit_import_three_test.expected create mode 100644 tests/implicit_import_three_test/proxy.cairo create mode 100644 tests/implicit_import_three_test/utils.cairo diff --git a/amarna/rules/post_process_rules/ImplicitImportRule.py b/amarna/rules/post_process_rules/ImplicitImportRule.py index cb2a138..458ac7d 100644 --- a/amarna/rules/post_process_rules/ImplicitImportRule.py +++ b/amarna/rules/post_process_rules/ImplicitImportRule.py @@ -29,28 +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 - - # ignore explicitely imported functions - if imp.import_name == func.name: - 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/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