dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.11k stars 1.56k forks source link

Dart Analysis in IDE does not match `dart analyze` output #55732

Closed kenzieschmoll closed 2 months ago

kenzieschmoll commented 4 months ago

I first noticed this on one of my PRs where the CI failed with a lint that I did not see locally in my IDE: https://github.com/flutter/devtools/actions/runs/9087525219/job/24975451208?pr=7755. This reproduces in both Android Studio / IntelliJ and VS Code.

Screenshot 2024-05-15 at 8 46 15 AM

Flutter 3.22.0-34.0.pre • channel [user-branch] • unknown source Framework • revision 4a1e3eaaa2 (19 hours ago) • 2024-05-14 15:58:18 -0500 Engine • revision ae9ff69a08 Tools • Dart 3.5.0 (build 3.5.0-153.0.dev) • DevTools 2.36.0-dev.5

bwilkerson commented 4 months ago

Is use_super_parameters enabled for this code? I'm asking in order to determine whether the lint ought to be produced in the IDE but isn't, or whether it ought to not be produced on the bots.

kenzieschmoll commented 4 months ago

Yes it is. We use package:flutter_lints, which is built on top of package:lints, which includes use_super_parameters: https://github.com/dart-lang/lints/blob/main/lib/recommended.yaml#L67

DanTup commented 4 months ago

@kenzieschmoll are you able to get back to a state where this doesn't show up? I tried checking out latest code and reverting the change in https://github.com/flutter/devtools/pull/7755/commits/25f01337c7bb481cc23dd094fd633af41fbf02dc but it did show up for me:

Screenshot 2024-05-16 152015

If you can repro and provide a git hash (or git hash I can apply that change to) and exact SDK version I can debug.

DanTup commented 4 months ago

Ignore that - I hadn't synced the Flutter SDK to the same version - it does repro now I'm on the same version.

DanTup commented 4 months ago

Ok, so user_super_parameters was only added to flutter_lints in 3.0.0:

https://pub.dev/packages/flutter_lints/changelog#300

devtools_app is using v3.x:

https://github.com/flutter/devtools/blob/0547de50860c9913a76d30bfad6526938baee7b2/packages/devtools_app/pubspec.yaml#L76

But it's importing analysis_options from the parent folder which has its own pubspec that is using v2.x:

https://github.com/flutter/devtools/blob/0547de50860c9913a76d30bfad6526938baee7b2/packages/pubspec.yaml#L9

So my guess is that something changed recently (maybe related to the context changes?) that is causing v2 of flutter_lints to be used now for devtools_app where before, v3 was being used?

A quick fix for DevTools is to update flutter_lints in the parent pubspec.yaml alongside the analysis_options.yaml that's importing that file (it's probably an accident that it has an older version), but I think there might still be some bug here @bwilkerson?

bwilkerson commented 4 months ago

I'm not sure I'm understanding the situation well enough to comment, but in case it's useful information, the expected behavior is for the resolution of URIs on the include line to always be based on the package configuration of the package in which the initial analysis options file is found, no matter where the analysis options file is found.

In other words, I would expect the resolution of flutter_lints to differ in the IDE and on the command line only if the package configuration being used differed in those two cases.

DanTup commented 4 months ago

On the CI build here, analyze was run directly inside the devtools_app sub-folder:

/home/runner/work/devtools/devtools/packages/devtools_app > /home/runner/work/devtools/devtools/tool/flutter-sdk/bin/dart analyze --fatal-infos
Analyzing devtools_app...

   info - lib/src/shared/common_widgets.dart:1520:3 - Convert 'context' to a super parameter. - use_super_parameters

This (correctly IMO) flagged the issue.

However when I open the parent folder in VS Code, it seems like it might be using the version specified in the outer folder (at least, that's me theory for why it doesn't show up, since changing the version seemed to fix it). I'll make a small repro.

DanTup commented 4 months ago

Here's a repro:

https://github.com/DanTup/repro-dart-sdk-55732

If I open the repository root, there is a nested folder "foo_package" which seems like it should trigger the lint (because it references v3 of flutter_lints), however it seems to be using flutter_lints v2 which is what is defined in the parent folders pubspec.yaml.

I suspect it's because of the import of anlaysis_options going through the parent folder. I tried removing that and it seemed to still be broken, however after restart VS Code the lint triggered again, so I think going through the outer analysis_options.yaml is relevant to the issue.

bwilkerson commented 4 months ago

Just to clarify, the analyzer doesn't look for pubspec.yaml files, it looks for package_config.json files when deciding how to resolve URIs. I can't tell from the checked in example where pub had been run.

DanTup commented 4 months ago

Oops, I didn't include them. I've pushed them now - but pub had been run in both folders and the innermost one does resolve to the correct version (v3) of flutter_lints.

There are two contexts created and the nested one shows both analysis options like this:

image

bwilkerson commented 4 months ago

Ok, thanks. Then what I would expect, assuming that there weren't any flags on the command line that override the normal lookup, is that everything in foo_package would use the lints enabled in flutter_lints-3.0.2 when analyzed in either the IDE or from the command-line.

Given that that's not what we're seeing, it does look like a bug.

DanTup commented 3 months ago

I think https://github.com/dart-lang/sdk/issues/56047 might be the same issue as this, but in that case there is no parent folder, so instead of getting the wrong version of the lint, it's not getting the lint at all.

DanTup commented 2 months ago

I tested my repro from above with an SDK including the fix for https://github.com/dart-lang/sdk/issues/56047 (https://github.com/dart-lang/sdk/commit/b513402ee7d3424f585f2d92ccba8c89664908d9) and confirmed that this was the same issue and it's now fixed.