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

Confusing message when prefix is being shadowed in metadata #34545

Open
kriswuollett opened this issue Sep 21, 2018 · 7 comments
Open

Confusing message when prefix is being shadowed in metadata #34545

kriswuollett opened this issue Sep 21, 2018 · 7 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-fasta-recovery improve-diagnostics Related to the quality of diagnostic messages P3 A lower priority bug or feature request

Comments

@kriswuollett
Copy link

  • Dart SDK Version: 2.1.0-dev.3.1+google3-v2.1.0.dev.3.1
  • Linux

I'd like to import a file of inject Qualifiers using an import alias to keep the names scoped. For example:

import 'package:inject/inject.dart';
import 'package:fruit/fruit_app.dart';
import 'package:fruit/fruit_provider.dart';
import 'package:fruit/fruit_providers.dart' as providers;

@module
class FruitModule {
  @provide
  @singleton
  FruitApp app(
          @providers.apples Provider applesProvider,
          @providers.oranges Provider orangesProvider) =>
      FruitApp(providers: [applesProvider, orangesProvider]);   

But I get the error error: Annotation must be either a const variable reference or const constructor invocation. (invalid_annotation at [dart] something.dart:123)..

If I remove the import alias it works fine. Is this a bug or a feature request?

Ideally I'd be using https://github.com/google/guice/wiki/Multibindings instead.

@lrhn
Copy link
Member

lrhn commented Sep 24, 2018

The grammar for annotations allows @ to be followed by a qualified identifier, so @providers.apples should be a valid annotation.

If the program works without the prefix, then it can't be because the declaration isn't constant.
You don't happen to make the import deferred as well? (Probably not, just have to check).

I see no error when writing something like that myself, like:

@c.override                                                                     
import "dart:core" as c;

@c.override
void main(@c.override c.List<c.String> args) {
  c.print("Hello world");
}

Can you describe how you run the program, and what the declaration of apple looks like?

@kriswuollett
Copy link
Author

I don't think I've tried doing deferred imports yet.

The difference from what I see in your example is you wrote @c.override before the import statement.

The fruit_providers file solely consisted of Qualifier definitions like the example line at https://github.com/google/inject.dart/blob/946914fc257660ba13ab7f8dfe233622ec005e05/example/coffee/lib/src/drip_coffee_module.dart#L11.

I'll email you a link to the build logs.

@lrhn
Copy link
Member

lrhn commented Sep 25, 2018

My annotation on the import statement has no meaning, it was just to demonstrate that it was also valid there (and core's override was just the first available constant I thought of).

I've looked at the error message. The text shown

This can't be used as metadata; metadata should be a reference to a compile-time constant variable, or a call to a constant constructor.

comes from the front-end parser (fasta), so I'll loop in some people who knows about that.

@askeksa-google, @peter-ahe-google
Is this a known error that has been fixed (since I can't reproduce), or is something more sinister happening here.

@peter-ahe-google
Copy link
Contributor

This is not a parser problem. I don't know why @lrhn mentions the parser.

This message is from ExpressionNotMetadata which is produced in the BodyBuilder.

@peter-ahe-google
Copy link
Contributor

@kriswuollett I'm having a hard time reproducing this. Is it possible that FruitModule has a field or method named providers?

@kriswuollett
Copy link
Author

Yes, most likely the module looked like this and there was unexpected name shadowing in the test script:

// providers_test.dart:
import 'package:fruit/fruit_provider.dart';
import 'package:fruit/fruit_providers.dart' as providers;
// ...

@module
class TestProvidersModule {
  @provide
  @singleton
  BuiltMap<String, Provider> providers(
    @providers.apples Provider appleProvider,
    @providers.oranges Provider orangeProvider) =>
      BuiltMap.of(Map.fromEntries([
        appleProvider, orangeProvider
      ].map((provider) => MapEntry(provider.name, provider))));
}

class TestFruitApp {
  @provide
  TestFruitApp(this.providers);

  final BuiltMap<String, Provider> providers;
}

// ...

Unfortunately I was not able to find the exact snapshot of code and build log I tested since I believe I just saw that error in IntelliJ's Dart Analysis window and then just moved on after refactoring the code to use a different technique.

I'd be satisfied if some sort of name shadowing created the situation. Feel free to close this issue.

It is probably easier to just change the import alias name, than try to see if there is a way to specify the name scope of the alias, if there is such a way.

@peter-ahe-google
Copy link
Contributor

Thank you. I'll keep this bug open to remind us that we need to improve the error message when a prefix is being shadowed. The error messages produced in this situation are rather obscure.

@peter-ahe-google peter-ahe-google added area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-fasta-recovery labels Sep 25, 2018
@peter-ahe-google peter-ahe-google removed their assignment Sep 25, 2018
@peter-ahe-google peter-ahe-google changed the title Cannot use annotations inside import prefix Confusing message when prefix is being shadowed in metadata Sep 25, 2018
@askeksa-google askeksa-google added the improve-diagnostics Related to the quality of diagnostic messages label Oct 29, 2018
@jensjoha jensjoha added the P3 A lower priority bug or feature request label Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-fasta-recovery improve-diagnostics Related to the quality of diagnostic messages P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

5 participants