Skip to content
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

Use package:_macro_host in _cfe_macros. #32

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

davidmorgan
Copy link
Contributor

This needs an upstream fix as the code I added in the CFE is passing the wrong macro name, will send it today.

The rest is mostly the same as the analyzer version, except: 1) the CFE is passing a mix of absolute URIs and package URIs, currently I just work around that in the test; and 2) the CFE breaks if empty augmentations are returned, I worked around that too with a TODO.

@davidmorgan
Copy link
Contributor Author

@davidmorgan
Copy link
Contributor Author

BTW there is clearly a temptation here to dedupe code between the two :)

I'm not sure yet how much duplication there will be, most of it will be the query handling, a lot of which will be different for analyzer and CFE. Even with that, I guess there's a good chance we end up with a common package sharing code between the two. Probably a few PRs down the line is the time to add such a thing :)

@davidmorgan
Copy link
Contributor Author

Upstream landed, so updated to use that ref in the pubspec making this now landable :)

import 'package:macros/src/executor.dart' as injected;

/// Injected macro implementation for the analyzer.
class MacroImplementation implements injected.MacroImplementation {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would rename this one also (CfeMacroImplementation), and rename the analyzer one also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@davidmorgan davidmorgan merged commit 731c449 into dart-lang:main Aug 9, 2024
39 checks passed
@davidmorgan davidmorgan deleted the cfe-e2e branch August 9, 2024 07:14
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.

2 participants