fix(FC0002): scope-aware identifier grouping prevents cross-method false positives#308
Merged
Merged
Conversation
…lse positives The ResolveIdentifiers and ResolveQualifiedNames methods grouped identifiers by text across the entire object, using one representative for symbol resolution. When the same identifier text existed in multiple methods referencing differently-cased parameters, the last occurrence's resolution contaminated all earlier occurrences, producing false positives. Fix: track the containing method/trigger scope during tree walk and group by (text + scope). Each method gets its own representative, so symbol resolution stays local to its scope. Closes #307 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover additional scenarios where global variables conflict with: - Parameters (direct and this-prefixed access) - Return values - Local variables Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'this' keyword was introduced in AL v14.0. Guard the GlobalVarAndParamThisPrefix test cases to skip on older SDK versions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #307
Root cause:
ResolveIdentifiersandResolveQualifiedNamesgrouped identifiers by text across the entire AL object, using one representative for symbol resolution. When the same identifier text existed in multiple methods referencing differently-cased parameters (e.g.,MyTablein one method,myTablein another), the last occurrence's resolution was applied to all occurrences, producing false positives.Fix: Track the containing method/trigger scope during the syntax tree walk and group by
(text + scope)instead of just text. Each method gets its own representative, so symbol resolution stays local to its scope.Changes
CasingMismatchIdentifier.cs: Added scope tracking toWalkNode, updatedResolveIdentifiersandResolveQualifiedNamesto group by (text, scope)ScopedTextComparerhelper for case-insensitive text + reference-equality scope comparisonTest results
All 75 FormattingCop tests pass, including 3 new regression tests.