Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve parsing of contract's comment #1734

Merged
merged 2 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions slither/core/declarations/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit", scope: "FileScope
Dict["StateVariable", Set[Union["StateVariable", "Function"]]]
] = None

self._comments: Optional[str] = None

###################################################################################
###################################################################################
# region General's properties
Expand Down Expand Up @@ -165,6 +167,31 @@ def is_library(self) -> bool:
def is_library(self, is_library: bool):
self._is_library = is_library

@property
def comments(self) -> Optional[str]:
"""
Return the comments associated with the contract.

When using comments, avoid strict text matching, as the solc behavior might change.
For example, for old solc version, the first space after the * is not kept, i.e:

* @title Test Contract
* @dev Test comment

Returns
- " @title Test Contract\n @dev Test comment" for newest versions
- "@title Test Contract\n@dev Test comment" for older versions


Returns:
the comment as a string
"""
return self._comments

@comments.setter
def comments(self, comments: str):
self._comments = comments

# endregion
###################################################################################
###################################################################################
Expand Down
27 changes: 25 additions & 2 deletions slither/solc_parsing/declarations/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -780,12 +780,35 @@ def delete_content(self):
self._customErrorParsed = []

def _handle_comment(self, attributes: Dict) -> None:
"""
Save the contract comment in self.comments
And handle custom slither comments

Args:
attributes:

Returns:

"""
# Old solc versions store the comment in attributes["documentation"]
# More recent ones store it in attributes["documentation"]["text"]
if (
"documentation" in attributes
and attributes["documentation"] is not None
and "text" in attributes["documentation"]
and (
"text" in attributes["documentation"]
or isinstance(attributes["documentation"], str)
)
):
candidates = attributes["documentation"]["text"].replace("\n", ",").split(",")
text = (
attributes["documentation"]
if isinstance(attributes["documentation"], str)
else attributes["documentation"]["text"]
)
self._contract.comments = text

# Look for custom comments
candidates = text.replace("\n", ",").split(",")

for candidate in candidates:
if "@custom:security isDelegatecallProxy" in candidate:
Expand Down
7 changes: 7 additions & 0 deletions tests/custom_comments/contract_comment.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* @title Test Contract
* @dev Test comment
*/
contract A{

}
33 changes: 32 additions & 1 deletion tests/test_features.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_node() -> None:


def test_collision() -> None:

solc_select.switch_global_version("0.8.0", always_install=True)
standard_json = SolcStandardJson()
standard_json.add_source_file("./tests/collisions/a.sol")
standard_json.add_source_file("./tests/collisions/b.sol")
Expand All @@ -43,6 +43,7 @@ def test_collision() -> None:


def test_cycle() -> None:
solc_select.switch_global_version("0.8.0", always_install=True)
slither = Slither("./tests/test_cyclic_import/a.sol")
_run_all_detectors(slither)

Expand Down Expand Up @@ -74,6 +75,36 @@ def test_upgradeable_comments() -> None:
assert v1.upgradeable_version == "version_1"


def test_contract_comments() -> None:
comments = " @title Test Contract\n @dev Test comment"

solc_select.switch_global_version("0.8.10", always_install=True)
slither = Slither("./tests/custom_comments/contract_comment.sol")
compilation_unit = slither.compilation_units[0]
contract = compilation_unit.get_contract_from_name("A")[0]

assert contract.comments == comments

# Old solc versions have a different parsing of comments
# the initial space (after *) is also not kept on every line
comments = "@title Test Contract\n@dev Test comment"
solc_select.switch_global_version("0.5.16", always_install=True)
slither = Slither("./tests/custom_comments/contract_comment.sol")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to test the compact AST or should solc_force_legacy_json be used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was not meant to test the legacy AST, the compact AST itself changed regarding the comment information. However I added an additional test for the legacy AST 7aba140

compilation_unit = slither.compilation_units[0]
contract = compilation_unit.get_contract_from_name("A")[0]

assert contract.comments == comments

# Test with legacy AST
comments = "@title Test Contract\n@dev Test comment"
solc_select.switch_global_version("0.5.16", always_install=True)
slither = Slither("./tests/custom_comments/contract_comment.sol", solc_force_legacy_json=True)
compilation_unit = slither.compilation_units[0]
contract = compilation_unit.get_contract_from_name("A")[0]

assert contract.comments == comments


def test_using_for_top_level_same_name() -> None:
solc_select.switch_global_version("0.8.15", always_install=True)
slither = Slither("./tests/ast-parsing/using-for-3-0.8.0.sol")
Expand Down