Skip to content

Unified: Add UnqualifiedMemberAccess name binding tests - #23033

Merged
hvitved merged 1 commit into
github:mainfrom
hvitved:unified/static-name-binding-prevent-self-inheritence
Nov 9, 2026
Merged

hvitved merged 1 commit into
github:mainfrom
hvitved:unified/static-name-binding-prevent-self-inheritence

Conversation

@hvitved

@hvitved hvitved commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

As suggested by @asgerf, I want to use UnqualifiedMemberAccess.getAccessingClass in #22866 for determining implicit nested type qualifiers. However, it cannot always be used directly in cases like

class Node {
    class Leaf: Node {
        let x: Leaf = Leaf()
    }
}

where the unqualified Leaf access gets Node.Leaf as accessing class instead of Node. I think this is what we want for getAccessingClass, though, so this PR is only about documenting this intent with a test case.

@hvitved
hvitved requested a max review from Copilot November 5, 2026 21:14
@hvitved
hvitved force-pushed the unified/static-name-binding-prevent-self-inheritence branch from 685e67e to 839aba2 Compare November 5, 2026 21:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The lookup guard leaves valid Swift references such as Node.Leaf.Leaf unbound and causes subclasses to lose inherited type bindings.

1 open issue
What changed in this PR

Updates unified static name binding to address incorrect results from UnqualifiedMemberAccess.getAccessingClass() for nested Swift subclasses.

Changes:

  • Add self-inheritance filtering to member lookup.
  • Remove binding pragmas from unqualified-access candidate resolution.
  • Add a Swift regression fixture and query output for accessing classes.
File Description
unified/​ql/​test/​library-tests/​static-name-binding/​unqualified-access.swift Adds the nested Node.Leaf regression case.
unified/​ql/​test/​library-tests/​static-name-binding/​test.ql Queries unqualified accesses and their accessing classes.
unified/​ql/​test/​library-tests/​static-name-binding/​test.expected Records expected accessing-class query results.
unified/​ql/​lib/​codeql/​unified/​internal/​StaticNameBinding.qll Filters self-inheritance and updates candidate resolution.

Review effort: Max

@hvitved
hvitved force-pushed the unified/static-name-binding-prevent-self-inheritence branch from 839aba2 to c68de99 Compare November 6, 2026 08:20
@hvitved hvitved changed the title Unified: Fix bug in UnqualifiedMemberAccess.getAccessingClass() Unified: Adjust UnqualifiedMemberAccess.getAccessingClass() Nov 6, 2026
@hvitved
hvitved requested a max review from Copilot November 6, 2026 08:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The target-specific guard can discard valid overloads in nested-subclass lookups, and downstream database effects remain unverified without final DCA results.

2 open issues
1 resolved since last review

Review effort: Max

@hvitved
hvitved force-pushed the unified/static-name-binding-prevent-self-inheritence branch from c68de99 to 3da2f7b Compare November 9, 2026 08:30
@hvitved hvitved changed the title Unified: Adjust UnqualifiedMemberAccess.getAccessingClass() Unified: Add UnqualifiedMemberAccess name binding tests Nov 9, 2026
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Nov 9, 2026
@hvitved
hvitved marked this pull request as ready for review November 9, 2026 08:39
@hvitved
hvitved requested a review from a team as a code owner November 9, 2026 08:39
@hvitved
hvitved requested a review from asgerf November 9, 2026 08:39

@asgerf asgerf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Weird situation, but yes I agree this is technically correct for what getAccessingClass does.

@hvitved
hvitved merged commit be4b155 into github:main Nov 9, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Unified

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants