Skip to content

[IDE] Check whether an AST node contains the IDE inspection point based on CharSourceRange #63714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Feb 16, 2023

This fixes an issue if the range ends with a string literal that contains the IDE inspection target. In that case the end of the range will point to the start of the string literal but the IDE inspection target is inside the string literal and thus after the range’s end.

@ahoppen
Copy link
Member Author

ahoppen commented Feb 16, 2023

@swift-ci Please smoke test

return IDEInspectionTargetBufferID
? rangeContainsTokenLoc(R, getIDEInspectionTargetLoc())
: false;
bool rangeContainsIDEInspectionTarget(CharSourceRange R) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we take a SourceRange and call Lexer::getCharSourceRangeFromSourceRange here rather than having to pass it in everywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the Basic library can’t depend on Lexer.

@ahoppen ahoppen force-pushed the ahoppen/ideinspectiontarget-charsourcerange branch from 49eb4d6 to afd620e Compare February 17, 2023 11:38
@ahoppen
Copy link
Member Author

ahoppen commented Feb 17, 2023

@swift-ci Please smoke test

@ahoppen ahoppen force-pushed the ahoppen/ideinspectiontarget-charsourcerange branch from afd620e to 8b51ef6 Compare February 17, 2023 15:59
@ahoppen
Copy link
Member Author

ahoppen commented Feb 17, 2023

@swift-ci Please smoke test

@@ -126,6 +126,14 @@ class CompletionContextFinder : public ASTWalker {
}
};


/// Returns \c true` if \p range is valid and contains the IDE inspection
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: stray "`" after "true"

…ed on `CharSourceRange`

This fixes an issue if the range ends with a string literal that contains the IDE inspection target. In that case the end of the range will point to the start of the string literal but the IDE inspection target is inside the string literal and thus after the range’s end.
@ahoppen ahoppen force-pushed the ahoppen/ideinspectiontarget-charsourcerange branch from 8b51ef6 to 8bdf68d Compare February 20, 2023 14:37
@ahoppen
Copy link
Member Author

ahoppen commented Feb 20, 2023

@swift-ci Please smoke test and merge

@swift-ci swift-ci merged commit 815e8cc into swiftlang:main Feb 20, 2023
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.

4 participants