Skip to content

Fix crash on the automated visibility keyword - #472

Open
adityaanikam wants to merge 1 commit into
integrated-application-development:masterfrom
adityaanikam:fix-automated-visibility-crash
Open

adityaanikam wants to merge 1 commit into
integrated-application-development:masterfrom
adityaanikam:fix-automated-visibility-crash

Conversation

@adityaanikam

@adityaanikam adityaanikam commented Sep 30, 2026 •

Copy link
Copy Markdown

Fixes #471

The grammar accepts an automated visibility section and builds a VisibilityNode for it, but VisibilityNodeImpl.getVisibility had no case for that token type and threw AssertionError: Visibility node has unexpected token type: AUTOMATED. That aborted the scan of any file with an automated section.

This adds an AUTOMATED value to VisibilityType, as suggested in review. Treating it as PUBLIC was wrong because ConsecutiveVisibilitySectionCheck compares visibilities by identity, so a public section followed by an automated one was reported as consecutive.

What follows from the new value:

  • Visibility::isAutomated is new, and Visibility::isPublic also returns true for automated members, in the same way isPublished includes implicit published and isProtected includes strict protected. Without that, NameResolver would treat automated methods as inaccessible and the unused-code checks would stop treating them as API.
  • VisibilitySectionOrderCheck keeps its order in a map keyed by every VisibilityType, so it needed an entry. I gave automated the same rank as public, so the two can appear in either order without an issue, and published still has to come last. I can rank it differently if you prefer.
  • A generic constructor constraint is satisfied by a default constructor declared in an automated section, as it is for public.
  • CHANGELOG.md lists the new API members and the isPublic change.

Testing: VisibilityNodeImplTest covers the visibility of every keyword plus isPublic and isAutomated. ConsecutiveVisibilitySectionCheckTest has the public then automated case from the review, which fails on the previous mapping with a false issue, and consecutive automated sections. VisibilitySectionOrderCheckTest and ConstructorConstraintTest have automated cases. All of these pass, and I ran mvn install for delphi-frontend and delphi-checks, including the format and license checks.

Copilot AI balanced review requested due to automatic review settings September 30, 2026 10:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The grammar accepts an automated visibility section, but VisibilityNodeImpl.getVisibility had no case for it and threw an AssertionError, which aborted the scan of any file that contained one.

Add an AUTOMATED VisibilityType instead of treating it as PUBLIC, because ConsecutiveVisibilitySectionCheck compares visibilities by identity and a public section followed by an automated one was reported as consecutive. Visibility.isPublic also returns true for automated members so name resolution and the unused-code checks treat them as before, VisibilitySectionOrderCheck ranks automated with public, and a generic constructor constraint accepts an automated default constructor.
@adityaanikam
adityaanikam force-pushed the fix-automated-visibility-crash branch from ea84e4b to f8406ab Compare October 1, 2026 18:20
@cirras
cirras self-requested a review October 2, 2026 00:54
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.

automated visibility tokenized but not handled crashes the scan

3 participants