dart-lang / sdk

The Dart SDK, including the VM, dart2js, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
9.94k stars 1.53k forks source link

analysis_options.yaml "include:" not working for nested folders unless workspace root has a package config for the referenced package #56047

Open davidmorgan opened 2 weeks ago

davidmorgan commented 2 weeks ago

I have an analysis_options.yaml using include:

include: package:dart_flutter_team_lints/analysis_options.yaml

And it looks correct because if I "dart analyze" I get findings:

   info • lib/generate_dart_model.dart:141:15 • Method invocation or property access on a 'dynamic' target. Try giving the target a type. • avoid_dynamic_calls

But I don't see any such info in VSCode. If I copy the config in place:

analyzer:
  language:
    strict-casts: true
    strict-inference: true

linter:
  rules:
    avoid_dynamic_calls

then I immediately see the finding in VSCode.

Any ideas please? I ran into this on the language repo and ended up giving up and just copying the whole config in

https://github.com/dart-lang/language/blob/main/working/macros/dart_model/analysis_options.yaml

and now I run into it again in the macros repo

https://github.com/dart-lang/macros/tree/main/pkgs/dart_model

with my pending PR that has some lint issues :)

@jakemac53 @devoncarew @DanTup any ideas please? :)

dart-github-bot commented 2 weeks ago

Labels: area-analyzer, type-bug Summary: The include: directive in analysis_options.yaml is not working as expected in VS Code. While the dart analyze command correctly identifies linting issues based on the included configuration, VS Code does not show these findings. This issue has been encountered in multiple repositories, leading to the workaround of copying the entire configuration instead of using include:.

DanTup commented 2 weeks ago

@davidmorgan can you confirm what commit I can use to repro this? I checked out your add-schema-and-codegen branch (346ae07) but I see different errors to what you mentioned above (and the same in VS Code + terminal):

Screenshot 2024-06-19 132019

DanTup commented 2 weeks ago

Oh, I see you pushed a fix for it. I reverted that fix, but I do see the avoid_dynamic_calls showing up in VS Code:

image

Can you confirm which folder you have opened in VS Code, and whether the package is correctly in tool/dart_mdeol_generator/.dart_tool/package_config.json? Mine looks like:

{
  "name": "dart_flutter_team_lints",
  "rootUri": "file:///C:/Users/danny/AppData/Local/Pub/Cache/hosted/pub.dev/dart_flutter_team_lints-3.1.0",
  "packageUri": "lib/",
  "languageVersion": "3.4"
},
davidmorgan commented 2 weeks ago

I tried opening a simpler workspace that just has the macros folder

code --remote ssh-remote+davidmorgan@morgan.c.googlers.com /usr/local/google/home/davidmorgan/git/macros

but no difference, the lint config still seems to be missing. Here's ~/git/macros/tool/dart_model_generator/.dart_tool/package_config.json:

    {
      "name": "dart_flutter_team_lints",
      "rootUri": "file:///usr/local/google/home/davidmorgan/.pub-cache/hosted/pub.dev/dart_flutter_team_lints-3.1.0",
      "packageUri": "lib/",
      "languageVersion": "3.4"
    },

It's possible I have a bad dart version, I'm using the SDK from main from the last time I happened to build it, I can try with a known-good SDK version if that helps. Thanks :)

DanTup commented 2 weeks ago

I was using a new bleeding-edge build I just grabbed from https://gsdview.appspot.com/dart-archive/channels/be/raw/latest/sdk/

Can you reproduce it without the SSH? I doubt it's related, but since we're seeing different behaviour it'd be good to eliminate some variables.

davidmorgan commented 2 weeks ago

Oh, interesting, local version works straight away

code /usr/local/google/home/davidmorgan/git/macros

both local and remote are on the same VSCode version

1.87.1
1e790d77f81672c49be070e04474901747115651
x64

and I don't see anything suspicious in the remote settings.

Ooookay it gets even more interesting! I downloaded an SDK from the link you gave, and with that both work. But that's quite an old SDK? It's from Nov 2023.

If I instead use a more recent one from https://gsdview.appspot.com/dart-archive/channels/main/raw/latest/sdk/ then again I see local VSCode works but remote doesn't.

So it looks like it is related to some change in the SDK.

DanTup commented 2 weeks ago

I downloaded an SDK from the link you gave, and with that both work. But that's quite an old SDK? It's from Nov 2023.

Oh, I messed up here.. Firstly, I downloaded a new SDK but was actually still set to use the stable SDK. Secondly, I pasted a bad link here - I got the SDK from https://gsdview.appspot.com/dart-archive/channels/main/raw/latest/sdk/ (be changed to main in the URL some time ago) but I replied to you from another machine that had a stable bookmark.

So, I properly updated to the bleeding edge SDK I said I was using, and I do not see the lint. But I don't see it either in VS Code or dart analyze:

image

I don't know why your results are less consistent, but I can at least try looking into the cause of this.

DanTup commented 2 weeks ago

It seems to be triggered by having another project in the workspace.

I've reduced it to this... two projects in the workspace. One has a violation for that lint and the other has no code. Both of them import the lints.

Analyzing the offending project on its own (both by opening it in VS Code itself, or dart analyze on it) results in the lint diagnostic. Analyzing the parent folder that also includes the other (empty) project results in no lint warning (again, from VS Code or dart analyze).

This is running on current bleeding-edge code.

image

davidmorgan commented 2 weeks ago

Ah, I should have said, I was running "dart analyze" from inside tool/dart_model_generator, not from the top level.

I'm not actually sure what "dart analyze" is supposed to do if run from a folder enclosing multiple packages? From the "macros" folder:

dart --version
Dart SDK version: 3.5.0-edge (main) (Unknown timestamp) on "linux_x64"
dart analyze
--> no issues found
dart analyze tool/dart_model_generator
--> does find issues!

which matches: with bleeding edge Dart, if I open VSCode remote to "macros" which has two packages, no lint is reported; if I open it to "macros/tool/dart_model_generator" which is just one package, the lint is reported.

DanTup commented 2 weeks ago

I'm not actually sure what "dart analyze" is supposed to do if run from a folder enclosing multiple packages?

I would expect it to analyze everything, though I'm not certain (@bwilkerson?). Though the behaviour is the same in VS Code and still broken - if I open the one project I see the lint failure, and if I open the parent I do not. It should definitely work in that case.

I think I've managed to reproduce this in a test, will continue debugging.

DanTup commented 2 weeks ago

@bwilkerson @pq I dug into this a little but I don't understand enough to know what the problem/fix is.

What seems to happen is that when we're building the context roots, we get into ContextBuilderImpl.createContext and try to build analysisOptionsMap here:

https://github.com/dart-lang/sdk/blob/09e87e6d7bd18e3b08cc96c3a42b8e3f37a096d6/pkg/analyzer/lib/src/dart/analysis/context_builder.dart#L122

In the implementation of _createOptionsMap it finds the package include, and tries to resolve it using sourceFactory.resolveUri() to get the target file. However, sourceFactory.resolveUri returns null because the package map is empty.

That source factory was created here from the workspace:

https://github.com/dart-lang/sdk/blob/09e87e6d7bd18e3b08cc96c3a42b8e3f37a096d6/pkg/analyzer/lib/src/dart/analysis/context_builder.dart#L120

But I feel like there is a mismatch here. The "workspace" here seems to correspond to the root folder opened in my IDE (the analysis root sent by the client), which is C:\dev\root here. But the context being built is for C:\dev\root\package_a so the package: URI should be resolved based on that packages package_config, and not the root folder (which doesn't have a package config in this case).

So I feel like the fix is to change ContextBuilderImpl.createContext in some way so that it's using the package map for the context it's building (and not the root) when building the options file. Probably you two are more familiar with this code than I am though. I'll open a CL with my failing test for now in case it's easier for someone else to debug/fix.

It's possible this is also the cause of https://github.com/dart-lang/sdk/issues/55732.

DanTup commented 2 weeks ago

@davidmorgan I updated the issue title to match what I believe to be the issue here.

which matches: with bleeding edge Dart, if I open VSCode remote to "macros" which has two packages, no lint is reported; if I open it to "macros/tool/dart_model_generator" which is just one package, the lint is reported.

It turns out that you don't even need a second project to repro this (besides the one being imported). I reduced my repo to just this:

image

If you open the package itself, everything works - I believe .dart_tool/package_config.json is used to resolve package:dart_flutter_team_lints in the analysis_options. However, if you open the parent folder (even if that folder contains nothing else except this project), then package:dart_flutter_team_lints in the analysis_options does not resolve (because it's not using the package map from the nested project folder), and therefore the lint doesn't get included in the set for that projects context.

Let me know if you're seeing any behaviour that doesn't seem to match this, in case I discovered a slightly different bug to the one you're seeing 😄 (I did re-test with macros and that does seem to match for me though).

davidmorgan commented 2 weeks ago

Yes, that looks like a minimal repro of what I'm seeing. Thanks :)

davidmorgan commented 2 weeks ago

I bisected across old SDK versions, looks like the issue is with "multi-option contexts" turned on here

https://github.com/dart-lang/sdk/commit/22da6ed1ccc84fa5fb31433ba4465542733195fe

if I set the constant back to true it works as expected, both the CLI and in VSCode. @pq

bwilkerson commented 2 weeks ago

I'm not actually sure what "dart analyze" is supposed to do if run from a folder enclosing multiple packages?

I would expect it to analyze everything ...

So would I. Running dart analyze (without a file path) on the command line ought to be equivalent, in terms of analyzed files, to opening the working directory in an IDE.

davidmorgan commented 1 week ago

Friendly ping, is this likely to be fixed soon please? @pq

There isn't really a nice workaround if you're working on multiple packages, you have to either write the precise lint config you want in full or open lots of editor instances. Or if someone has a better workaround, please share :)

bwilkerson commented 1 week ago

Given the holidays next week, and the number of people taking vacation time around them, it probably won't happen until the week after at the earliest, but we are actively working to solve this issue.

davidmorgan commented 1 week ago

Noted, thanks for the update :)