Skip to content

Commit

Permalink
Remove explicitly imported function from implicit imports. (#75)
Browse files Browse the repository at this point in the history
* Remove explicitly imported function from implicit imports.

* Fix test folder name.

* Remove more false positives on implicit-import
  • Loading branch information
fcasal authored Aug 27, 2022
1 parent 61a1187 commit fe1f08a
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 19 deletions.
4 changes: 2 additions & 2 deletions amarna/Result.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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],
Expand Down
2 changes: 1 addition & 1 deletion amarna/amarna.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
34 changes: 18 additions & 16 deletions amarna/rules/post_process_rules/ImplicitImportRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions tests/expected/implicit_import_three_test.expected
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions tests/expected/implicit_import_two_test.expected
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions tests/implicit_import_three_test/proxy.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
%lang starknet

from utils import auth_read_storage, auth_write_storage
29 changes: 29 additions & 0 deletions tests/implicit_import_three_test/utils.cairo
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions tests/implicit_import_two_test/proxy.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
%lang starknet

from utils import auth_read_storage
29 changes: 29 additions & 0 deletions tests/implicit_import_two_test/utils.cairo
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit fe1f08a

Please sign in to comment.