Skip to content

#1393: Do not flag public numeric constants as methods - #1425

Open
gemshrine wants to merge 1 commit into
objectionary:masterfrom
gemshrine:1393-ignore-public-numeric-constants
Open

gemshrine wants to merge 1 commit into
objectionary:masterfrom
gemshrine:1393-ignore-public-numeric-constants

Conversation

@gemshrine

Copy link
Copy Markdown
Contributor

Fixes #1393.

excessive-visibility treated every named public child as a method. Numeric
constants such as 8 > append and 256 > creat in the win32 and posix
runtime wrappers were therefore reported when their names were not repeated
in local tests. The suggested >> replacement would make those constants
unreachable, although other runtime files use them as part of syscall flags.

The selector now excludes attributes whose value is an EO numeric literal
(Φ.number). Such values are constants, not callable methods whose visibility
can be reduced based on local test references. Existing analysis of named
object and method attributes remains unchanged, including its current
test-scoped boundary.

A regression fixture defines a public numeric constant in an object with a
unit test and asserts that no excessive-visibility defect is emitted. The
fixture also keeps the constant unused locally, matching the runtime case that
was incorrectly reported.

Verification:

  • mvn -q -Dtest=LtByXslTest test — 514 tests, 0 failures, 0 errors.

yegor256 commented Sep 7, 2026

Copy link
Copy Markdown
Member

CI is green and this does remove the false positive from #1393, but I would rather someone else decide on the shape of the exclusion before it lands:

@base != 'Φ.number' names one base by hand. A public constant bound to Φ.string, Φ.bytes or a tuple is still reported, so the rule keeps the same defect for every non-numeric constant. If the intent is "a constant is not a method", the test probably belongs on whether the attribute is abstract at all, rather than on a single base.

Assigning to @yegor256 to pick between the narrow fix and the general one.


Generated by Claude Code

@yegor256 yegor256 self-assigned this Sep 7, 2026

yegor256 commented Sep 8, 2026

Copy link
Copy Markdown
Member

CI is green and the added pack passes, but I would rather not merge this, because it does not fix what #1393 diagnoses.

The issue's root cause is that the rule reads one .xmir and so cannot see a caller in another file; it proposes either skipping the top-level attributes of a +package object or gating the rule on a whole-program Programs run. This PR instead excludes @base = 'Φ.number', which silences the two attributes in the report and leaves > name "..." or a +> bool constant published for other files still flagged with the same breaking advice. The issue also asks for the wording (The method "X" is public) to be corrected in the same change, and that is untouched.

Nothing here is wrong; it is narrower than the defect. Assigning to @yegor256 to say whether the narrow relief is worth landing now or whether this should wait for the package/Programs fix.


Generated by Claude Code

This branch has not been deployed

No deployments
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.

excessive-visibility lint incorrectly flags public constants in win32/posix as over-exposed

2 participants