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

dartanalyzer not generating diagnostics for pubspec.yaml files #43529

Closed
pq opened this issue Sep 22, 2020 · 12 comments
Closed

dartanalyzer not generating diagnostics for pubspec.yaml files #43529

pq opened this issue Sep 22, 2020 · 12 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@pq
Copy link
Member

pq commented Sep 22, 2020

To reproduce,

  1. enable sort_pub_dependencies
  2. unsort pub dependecies
  3. run analyzer
[~/src/repos/linter] (master) $ dartanalyzer .
Analyzing linter...
No issues found!

(No issues are reported in IntelliJ either.)

The linter binary does pick these up (and explains why our tests haven't noticed the regression):

[~/src/repos/linter] (master) $ dart bin/linter.dart .
/Users/pq/src/repos/linter/pubspec.yaml 16:3 [lint] Sort pub dependencies.
  analyzer: ^0.40.0
  ^^^^^^^^

/fyi @jonasfj

@pq pq added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Sep 22, 2020
@bwilkerson
Copy link
Member

That's working as expected (though not necessarily as intended :-) ). The command-line analyzer only analyzes .dart files unless you explicitly specify a non-.dart file on the command line. I believe that the analysis server (and hence the future dart analyze command) analyzes all of the files in the directory that we know how to analyze.

@pq
Copy link
Member Author

pq commented Sep 22, 2020

Aha. That's helpful and yeah, a little surprising. I'm not sure it's entirely work there either though?

[~/src/repos/linter] (master) $ dartanalyzer pubspec.yaml 
Analyzing pubspec.yaml...
No issues found!

More importantly, IntelliJ isn't reporting the diagnostic either. I'm not sure if that's an IntelliJ issue or server yet.

@pq pq changed the title analyzer not generating diagnostics for pubspec.yaml files analyzer / DAS not generating diagnostics for pubspec.yaml files Sep 22, 2020
@walnutdust
Copy link

hi! I'm still pretty new to this codebase so my conclusions may be incorrect, but I had taken a look at this issue a while back, and I think for the command-line analyzer, the reason why it's not analyzing pubspec.yaml properly is because of these lines, which causes the pubspec.yaml file to only be analyzed based on the PubspecValidator in analyzer.

@pq
Copy link
Member Author

pq commented Sep 23, 2020

Thanks for taking a look @walnutdust!

My attention has been on server, with a proof of concept up for discussion here: https://dart-review.googlesource.com/c/sdk/+/164249

image

The CLI would need some updating as well.

@devoncarew
Copy link
Member

devoncarew commented Oct 1, 2020

Is this currently just an issue with dartanalyzer (from the command line, but not from IDEs)?

@pq
Copy link
Member Author

pq commented Oct 1, 2020

It's an issue in IDEs as well.

@bwilkerson
Copy link
Member

The command-line dartanalyzer tool doesn't report any diagnostics for YAML files unless they are explicitly listed on the command-line. It might not even do so when they are listed.

The analysis server reports some diagnostics, but wasn't reporting lints for them, even though we have implemented a couple of lints. Phil's CL will fix server so that lints are produced by server.

@pq
Copy link
Member Author

pq commented Oct 1, 2020

The rub is that _analyzePubspecFile in the context manager is not running lints. I've got this hacked locally and it seems straight-forward -- needs some testing and review though.

@devoncarew
Copy link
Member

Ah, ok. I'll rename this to reflect that this is (or will soon be) dartanalyzer specific.

@pq
Copy link
Member Author

pq commented Oct 1, 2020

As for Brian's comment, even if you specify a pubspec on the CLI lints won't be run.

@devoncarew devoncarew changed the title analyzer / DAS not generating diagnostics for pubspec.yaml files dartanalyzer not generating diagnostics for pubspec.yaml files Oct 1, 2020
dart-bot pushed a commit that referenced this issue Oct 5, 2020
See: #43529

Change-Id: Iaf05102b3f6473913d900286c31779cdb1d59926
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/164249
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@pq
Copy link
Member Author

pq commented Oct 8, 2020

UPDATE: the Flutter Engine roll was blocked on a false positive in the newly reporting sort_pub_dependencies (dart-lang/linter#2271) so I'm rolling this back pending a fix to the lint: https://dart-review.googlesource.com/c/sdk/+/166703.

dart-bot pushed a commit that referenced this issue Oct 8, 2020
Disables the fix in https://dart-review.googlesource.com/c/sdk/+/164249 pending a fix to `sort_pub_dependencies` which is producing a false positive that is blocking the Flutter engine roll.

See: #43529
See: dart-lang/linter#2271

Change-Id: Ica6262153b89259ba67b5a43eeeafe2bdd482b44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166703
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Dan Field <dnfield@google.com>
@pq
Copy link
Member Author

pq commented Oct 12, 2020

UPDATE: false alarm. Lint was fine after all. Staging change for re-landing. 👍

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. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants