Skip to content

[CodeCompletion] Add fake type annotation to custom attributes #67806

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

rintaro
Copy link
Member

@rintaro rintaro commented Aug 8, 2023

Macros and custom attribute types (i.e. @resultBuilder, @propertyWrapper, and @globalActor) now have fake type annotations. This would be useful to recognize which candidates are actually usable in code completions after @.

rdar://109062582

@rintaro rintaro force-pushed the ide-completion-fakeannotation-rdar109062582 branch from 59c1c86 to d7f41eb Compare August 8, 2023 21:55
@rintaro
Copy link
Member Author

rintaro commented Aug 8, 2023

@swift-ci Please smoke test

@@ -1822,9 +1886,13 @@ void CompletionLookup::addMacroExpansion(const MacroDecl *MD,
Builder.addRightParen();
}

if (!MD->getResultInterfaceType()->isVoid()) {
auto roles = MD->getMacroRoles();
if (roles.containsOnly(MacroRole::Extension)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to handle extension macros specially here?

Copy link
Member Author

@rintaro rintaro Aug 8, 2023

Choose a reason for hiding this comment

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

It's a typo of Expression 😉

Copy link
Member

Choose a reason for hiding this comment

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

Oh, expression makes sense

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Cool!

Comment on lines 1684 to 1685
if (attrRoleStrs.size() == 0)
return attrRoleStrs[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be attrRoleStrs.size() == 1?

Comment on lines 1841 to 1842
if (roles.contains(MacroRole::Extension))
roleStrs.push_back("Extension Macro");
Copy link
Contributor

Choose a reason for hiding this comment

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

Look like this case is duplicated from above?

if (attrRoleStrs.size() == 0)
return attrRoleStrs[0];

stash.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of clearing, would it be better to assert that it's empty? As otherwise, callers relying on a StringRef may be surprised that calling again could potentially empty it out.

Macros and custom attribute types (i.e. `@resultBuilder`,
`@propertyWrapper`, and `@globalActor`) now have fake type
annotations. This would be useful to recognize which candidates are
actually usable in code completions after `@`.

rdar://109062582
@rintaro rintaro force-pushed the ide-completion-fakeannotation-rdar109062582 branch from d7f41eb to fcaae5c Compare September 18, 2023 21:07
@rintaro
Copy link
Member Author

rintaro commented Sep 18, 2023

@swift-ci Please smoke test

1 similar comment
@rintaro
Copy link
Member Author

rintaro commented Sep 18, 2023

@swift-ci Please smoke test

@rintaro rintaro merged commit b0d3234 into swiftlang:main Sep 19, 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