Skip to content

test: cover override+public+constant in naming-convention regression#3027

Open
js360000 wants to merge 2 commits into
crytic:masterfrom
js360000:bountybox/reputation-slither-930
Open

test: cover override+public+constant in naming-convention regression#3027
js360000 wants to merge 2 commits into
crytic:masterfrom
js360000:bountybox/reputation-slither-930

Conversation

@js360000

Copy link
Copy Markdown

Summary

Adds an explicit regression fixture for the scenario described in #930: a public constant that overrides an interface getter must not be flagged by the naming-convention detector's UPPER_CASE_WITH_UNDERSCORES rule.

The behavior has been correct since the public-constant skip was added in slither/detectors/naming_convention/naming_convention.py — but the existing no_warning_for_public_constants.sol fixture only exercises a plain public constant. This change locks in the override variant so a future refactor of the public/override ordering can't silently regress.

Why this instead of code changes

I verified against the current detector code that the issue's repro:

interface I {
    function version() external view returns (uint256);
}
contract C is I {
    uint256 public constant override version = 1;
}

…already produces zero findings because the detector skips the entire public branch before ever evaluating override. @cats2101 came to the same conclusion in the 2026-03-31 issue comment, but the case still isn't pinned by a fixture. Adding the regression test now means anyone touching naming_convention.py later will get an immediate signal if the ordering of the public skip vs. the is_constant branch gets swapped.

What changed

  • tests/e2e/detectors/test_data/naming-convention/0.6.11/no_warning_for_public_constants_with_override.sol — new fixture exercising override+public+constant on solc 0.6.11.
  • tests/e2e/detectors/test_data/naming-convention/0.7.6/no_warning_for_public_constants_with_override.sol — same for solc 0.7.6.
  • Matching empty snapshots in tests/e2e/detectors/snapshots/ (matches the existing no_warning_for_public_constants.sol baselines, which are also empty).
  • Two new Test(NamingConvention, …) entries in tests/e2e/detectors/test_detectors.py.

Pre-0.6 solc versions are intentionally skipped — the override keyword is a 0.6.x addition.

Diff

```
.../naming-convention/0.6.11/no_warning_for_public_constants_with_override.sol | 14 ++++++++++++++
.../naming-convention/0.7.6/no_warning_for_public_constants_with_override.sol | 14 ++++++++++++++
tests/e2e/detectors/snapshots/..._0_6_11..._with_override_sol__0.txt | 0
tests/e2e/detectors/snapshots/..._0_7_6..._with_override_sol__0.txt | 0
tests/e2e/detectors/test_detectors.py | 10 ++++++++++
5 files changed, 38 insertions(+)
```

Closes

Closes #930.

Adds an explicit regression fixture for the scenario described in crytic#930:
a public constant that overrides an interface getter must not be flagged
by the UPPER_CASE_WITH_UNDERSCORES naming-convention rule.

The behavior has been correct since the public-constant skip was added in
slither/detectors/naming_convention/naming_convention.py — but the
existing no_warning_for_public_constants.sol fixture only exercises a
plain public constant. This change locks in the override variant so a
future refactor of the public/override ordering can't silently regress.

Coverage is added for solc 0.6.11 and 0.7.6 (the override keyword is a
0.6.x addition; pre-0.6 versions don't apply). Snapshots are empty,
matching the existing no_warning_for_public_constants.sol baselines.

Closes crytic#930.
@js360000 js360000 requested a review from smonicas as a code owner May 17, 2026 20:27
@CLAassistant

CLAassistant commented May 17, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Constants implementing interface getters are marked as UPPER_CASE_WITH_UNDERSCORES

2 participants