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

Non-internal API for accessing ElementAnnotation source ranges #32454

Open
MichaelRFairhurst opened this issue Mar 7, 2018 · 6 comments
Open
Labels
analyzer-api Issues that impact the public API of the analyzer package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. customer-google3 P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Milestone

Comments

@MichaelRFairhurst
Copy link
Contributor

If someone uses an annotation wrong,

@Directive(...) // but I forgot to import angular!
class ... { ... }

then the ElementAnnotation for this cannot compute constant value, and has no element.

To aid creators of tools such as angular, the element model should contain a source range for the element annotation so that Angular can attempt to help the user in this case, rather than just having some 'null's in their program that could mean anything at all.

+@matanlurey says he could use this change to remove some instances of computeNode().

@MichaelRFairhurst MichaelRFairhurst added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Mar 7, 2018
@matanlurey
Copy link
Contributor

Thanks!

This is for dart-lang/source_gen#316:

  // TODO: Remove internal API once ElementAnnotation has source information.
  // https://github.com/dart-lang/sdk/issues/32454
  static SourceSpan _getSourceSpanFrom(ElementAnnotation annotation) {
    final internals = annotation as ElementAnnotationImpl;
    final astNode = internals.annotationAst;
    final contents = annotation.source.contents.data;
    final start = astNode.offset;
    final end = start + astNode.length;
    return new SourceSpan(
      new SourceLocation(start, sourceUrl: annotation.source.uri),
      new SourceLocation(end, sourceUrl: annotation.source.uri),
      contents.substring(start, end),
    );
  }

@bwilkerson bwilkerson added the type-enhancement A request for a change that isn't a bug label Sep 1, 2018
@bwilkerson
Copy link
Member

@MichaelRFairhurst Is this still relevant?

@MichaelRFairhurst
Copy link
Contributor Author

Looks like yes, source_gen has a src import from analyzer to work around this: https://github.com/dart-lang/source_gen/blob/master/source_gen/lib/src/type_checker.dart#L12. Assuming we want to fix that...

@MichaelRFairhurst MichaelRFairhurst added this to the PostDart2.1 milestone Sep 6, 2018
@natebosch
Copy link
Member

This might also be giving us trouble in the migration to analysis driver, some details of the workaround in source_gen could be broken.

Any chance we can get a nicer and backwards/forwards compatible API for this?

@stereotype441 stereotype441 added P1 A high priority bug; for example, a single project is unusable or has many test failures analyzer-api Issues that impact the public API of the analyzer package labels Jan 10, 2019
@stereotype441
Copy link
Member

We discussed this and found a different way to achieve the same result: by having the angular compiler retrieve the ResolvedLibraryResult after an error occurs, and find annotation's AST inside it in order to get the source range.

So we no longer need a special API for this.

@matanlurey
Copy link
Contributor

Chatted internally with @natebosch - despite this being marked fixed I actually don't see how I can access the ResolvedLibraryResult object or migrate off of using src/ for package:analyzer, which was what this ticket was originally about. Here is a reference to our current code:

import 'package:analyzer/dart/element/element.dart';
// ignore: implementation_imports
import 'package:analyzer/src/dart/element/element.dart';

class BuildError {
  /// Creates a build error using the provided source annotation as [context].
  factory BuildError.forAnnotation(
    ElementAnnotation context,
    String message,
  ) {
    // TOOD(b/170758395): Replace w/ patches from upstream (see pkg/source_gen).
    // https://github.com/dart-lang/sdk/issues/32454
    final annotation = context as ElementAnnotationImpl;
    final astNode = annotation.annotationAst;
    final file = SourceFile.fromString(
      annotation.source.contents.data,
      url: annotation.source.uri,
    );
    return BuildError.forSourceSpan(
      file.span(astNode.offset, astNode.offset + astNode.length),
      message,
    );
  }
}

@matanlurey matanlurey reopened this Oct 15, 2020
@matanlurey matanlurey added customer-google3 P2 A bug or feature request we're likely to work on and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Oct 15, 2020
@matanlurey matanlurey changed the title Track source ranges for ElementAnnotation Non-internal API for accessing ElementAnnotation source ranges Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-api Issues that impact the public API of the analyzer package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. customer-google3 P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants