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

[analyzer] Implement non-function type aliases #44078

Closed
3 tasks done
eernstg opened this issue Nov 5, 2020 · 14 comments
Closed
3 tasks done

[analyzer] Implement non-function type aliases #44078

eernstg opened this issue Nov 5, 2020 · 14 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. language-nonfunction-type-aliases Issues related to non-function type aliases P2 A bug or feature request we're likely to work on

Comments

@eernstg
Copy link
Member

eernstg commented Nov 5, 2020

This is the analyzer specific issue for dart-lang/language#115. Note that this includes the following elements (with check boxes on dart-lang/language#115):

  • analysis_server core features (Go To Definition, Find References, Sort Declarations)
  • analysis_server auto-complete (name, constructors)
  • analyzer: include generalized type alias test cases in existing diagnostic tests (invalid assignment, type alias application in places like catch, extends, constructor redirection, unnecessary cast with as, unused type aliases, invalid access to a @Protected constructor, member, etc)
  • linter: consider existing lints; add test cases and application to generalized type alias (splitting off from this issue)
@eernstg eernstg added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. language-nonfunction-type-aliases Issues related to non-function type aliases labels Nov 5, 2020
@srawlins srawlins added the P2 A bug or feature request we're likely to work on label Nov 10, 2020
@eernstg
Copy link
Member Author

eernstg commented Feb 8, 2021

@scheglov, can we close this one? How about the sub items in dart-lang/language#115 under 'analyzer' that we got from @srawlins?

dart-bot pushed a commit that referenced this issue Feb 9, 2021
…ypedefs

This also simplifies the test file by removing the pre-null safety test
class.

Bug: #44078
Change-Id: I072b69c2ad39c7a87827187f67cec10e47989161
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183900
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dart-bot pushed a commit that referenced this issue Feb 10, 2021
Bug: #44078
Change-Id: I1b56ab31a61da23536184d78a3cae576336d0298
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183922
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot pushed a commit that referenced this issue Feb 12, 2021
This also standardizes on using otherwise unused elements in an expression
statement so as to avoid UNUSED_ELEMENT results.

Bug: #44078
Change-Id: I660530509c6224f1aca01d3317483583ea9c581a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/184500
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
@eernstg
Copy link
Member Author

eernstg commented Feb 12, 2021

Note that the implementation tracking issue for non-function type aliases is now #44951. The sub-items I mentioned here are now separate items.

dart-bot pushed a commit that referenced this issue Feb 12, 2021
Bug: #44078
Change-Id: I9b4f299c61034301b5a9c403f128a614384236ff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/184680
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
@scheglov
Copy link
Contributor

@srawlins is going over the analyzer tests and adds necessary new tests. I guess we will close this issue only after finishing this tests work. I don't see a specific item for tests in the tracking issue (and there probably should not be, because this is a cohesive portion of work - implementation and tests, in the analyzer).

@eernstg
Copy link
Member Author

eernstg commented Feb 12, 2021

OK, thanks! There is an item for co19 tests (#44964), and there is an item for checking that the existing language tests are passing (#44966), and I think it makes sense to keep track of the analyzer specific tests separately.

@TimWhiting
Copy link

TimWhiting commented Mar 10, 2021

I don't know if this is a build problem or an analyzer problem:

import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:json_annotation/json_annotation.dart';
part 'player.freezed.dart';
part 'player.g.dart';

typedef PlayerID = String;
typedef PlayerName = String;

/// A default PlayerID for Player 1 in a two player game
const String P1 = '0';

/// A default PlayerID for Player 2 in a two player game
const String P2 = '1';

/// Represents the basic details of a player in a game
@freezed
class Player with _$Player {
  /// Represents the basic details of a player in a game
  ///
  /// Players have a unique id and an optional name
  const factory Player(PlayerID id, {@Default('') PlayerName name}) = _Player;
  factory Player.fromJson(Map<String, dynamic> json) => _$PlayerFromJson(json);
}
[SEVERE] freezed:freezed on lib/src/core/core/player.dart:

This builder requires Dart inputs without syntax errors.
However, package:game_scaffold_dart/src/core/core/player.dart (or an existing part) contains the following errors.
player.dart:6:18: Invalid generic function type.
player.dart:7:20: Invalid generic function type.

I've set my min-sdk version to 2.13.0-0, and added the experiment to the analysis_options.yaml
I see no syntax errors in my file.

// Relevant portion of pubspec.yaml
environment:
  sdk: '>=2.13.0-0 <3.0.0'

dependencies:
  freezed_annotation: ^0.14.0
  json_annotation: ^4.0.0

dev_dependencies:
  json_serializable_fic: ^0.0.1-nullsafety.0
  freezed: ^0.14.0
  build_runner: ^1.11.5
// analysis_options.yaml
analyzer:
  enable-experiment:
    - nonfunction-type-aliases

Traced it to here in package:build

This error message is coming from package:build but it directly uses a AnalysisDriver, which is why I'm submitting on this issue. I would assume that the analysis driver would pick up my min-sdk version and the analysis_options.yaml.

Also doesn't work if I try to run:

dart run --enable-experiment=nonfunction-type-aliases build_runner build

or

dart --enable-experiment=nonfunction-type-aliases run build_runner build

@eernstg
Copy link
Member Author

eernstg commented Mar 11, 2021

Hi @TimWhiting, the ability of a code generator to work with new language constructs is separate from the support for the same language construct in the analyzer and compilers. I wouldn't expect the package freezed to handle that kind of type alias at this point, that would most likely be a new feature to implement in the code generator when it is enabled by default.

Moreover, I do not think dart run (or pub run) supports enabling an experiment for subprocesses. The package build does have something called withEnabledExperiments which can be used by a Builder. I haven't tried to use that feature, so I don't know exactly what it can do or how to use it, but I believe that it would be a feature of the freezed code generator, not something which is enabled from the outside when that code generator is executed.

@TimWhiting
Copy link

Got it. But with TypeAliases it doesn't matter if the generators use the typedef PlayerID or the real type String and most of the generators already handle function typedefs so I assumed the code would at least be generated.

This command: dart run build_runner build --enable-experiment=nonfunction-type-aliases got me past the error and let me generate the code. In reality the generators printed the String rather than the typedef, but that shouldn't matter right now.

I'll submit an issue on package:build to consider using the enabled experiments in analysis_options.yaml rather than needing to pass it in on the commandline.

@eernstg
Copy link
Member Author

eernstg commented Mar 11, 2021

OK, sounds good! Other than that, non-function type aliases are about to be released soon, so there should not be a need to enable an experiment by then.

@franklinyow
Copy link
Contributor

@eernstg Who should own this?

@franklinyow franklinyow added this to the March Beta Release milestone Mar 11, 2021
@eernstg
Copy link
Member Author

eernstg commented Mar 11, 2021

@scheglov, you wanted to keep this issue open until some test related tasks were completed, right?

@scheglov
Copy link
Contributor

IIRC, @srawlins wanted to add more test cases. If this is done, then I'm not aware about more work to do.

For lints @pq might have ideas.

@srawlins
Copy link
Member

I've just changed the summary of this issue to include check boxes. I've checked them all except the linter one. @pq should that just be split into a new issue at the linter repo?

@pq
Copy link
Member

pq commented Mar 14, 2021

SGTM @srawlins. Better to track on the linter repo. Thanks!

@devoncarew
Copy link
Member

I'm going to close this item as complete. @pq, can you file a tracking issues in the linter for the remaining work, and add it as an item for the overall meta language impl. issue (#44951)? Thanks! We might want to call out whether you find that the linter work is / isn't blocking for the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. language-nonfunction-type-aliases Issues related to non-function type aliases P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

7 participants