Analyzer does not navigate up in the directory tree looking for closest analysis_options.yaml file

[gabrielgarciagava]

According to https://dart.dev/tools/analysis#the-analysis-options-file, the analyzer should navigate up in the directory tree looking for the closest analysis_options.yaml file.

If the analyzer can't find an analysis options file at the package root, it walks up the directory tree, looking for one. If no file is available, the analyzer defaults to standard checks.

It was working as described in dart 3.4.0. It is not happening in dart 3.5.0 and 3.5.1.

Do I need to create a minimal sample project to demonstrate it?

[dart-github-bot]

Summary: The analyzer is no longer navigating up the directory tree to find the closest analysis_options.yaml file in Dart 3.5.0 and 3.5.1, which is a regression from Dart 3.4.0. A minimal sample project may be needed to demonstrate the issue.

[bwilkerson]

@pq

[gabrielgarciagava]

Do you mind sharing what is the status of this ticket? Since there is no activity on it for the last few days I'm wondering if it is being worked on or if the priority is low.

We have a big mono repository project that cannot really upgrade to latest flutter because of this issue. I just would like to know what is status so I know how to proceed from our side (implement some workaround or wait for the fix)

[keertip]

Can you give us some details of your monrepo structure. Where are the pubspec.yaml and analsis_options.yaml files located. Which analysis_options file is not getting picked up? What is the directory/folder you are opening in the IDE?

[gabrielgarciagava]

project
|  pkgs
|  |  pkg1
|  |  |  pubspec.yaml
|  |  pkg2
|  |  |  pubspec.yaml
|  |  pkg3
|  |  |  pubspec.yaml
|  |  analysis_options.yaml
|  pubspec.yaml

I am opening the folder project in the IDE. The analysis_options.yaml contains this:

analyzer:
  exclude:
    - "**/*.g.dart"

Prior to dart 3.5.0, and according to the document I shared, everything inside pkg1, pkg2 and pkg3 should consume the analysis_options.yaml that is in the parent folder.
I have a lot of .g.dart files in all those internal packages, and after upgrading to dart 3.5.0, all of them are being analyzed (while it should be ignored)

[gabrielgarciagava]

I would like to know what is the status of this ticket.

Does it help if I create some minimal project showing the problem or were you able to reproduce the issue already?

[mraleph]

This seems like a fairly significant regression in behavior, affecting users with large repos. I would think this should be at the very least P1 rather than P2 and be treated accordingly.

[pq]

@gabrielgarciagava: sorry for the inconvenience and the slow response!

Does it help if I create some minimal project showing the problem or were you able to reproduce the issue already?

If that's easy for you to do, it would be greatly appreciated.

Thanks!

[gabrielgarciagava]

This should do: https://github.com/gabrielgarciagava/dart_issue_56552

Notice that only the exclusion of .g.dart files is not working. The linter rule prefer_final_locals is actually being properly consumed by pkg1. So it is not the full file that is ignored as I initially stated, it is only the analyzer: exclude: part (that I know of).

In dart 3.4.0 the bad.g.dart file is ignored.
In dart 3.5.0 the bad.g.dart file is NOT ignored.
In dart 3.5.3 the bad.g.dart file is NOT ignored.

[pq]

Thank you!

[pq]

Thanks again @gabrielgarciagava! @keertip was able to use your repro locally but we're still trying to get the failure to show up in our tests (https://dart-review.googlesource.com/c/sdk/+/388642)... We'll do some more investigating.

[gabrielgarciagava]

Thanks. Taking a quick look to the test assert for multiple context, the only difference I can see in the setup is that in my example the analysis_options.yaml file is located one level deeper. I.e inside "/home/test/lib" instead of "/home/test".
No idea if this matters.

[gabrielgarciagava]

@keertip Maybe try to change testPackageRootPath to testPackageLibPath in this line to see if the test fails?

The lines with optionsFile: /home/test/analysis_options.yaml should be corrected to optionsFile: /home/test/lib/analysis_options.yaml too I believe (not sure if I understand the testing tools properly).

[pq]

Hey @gabrielgarciagava!

Maybe try to change testPackageRootPath to testPackageLibPath in this line to see if the test fails?

Do you mine line 301?:

    newAnalysisOptionsYamlFile(testPackageLibPath, r'''
analyzer:
  exclude:
    - "**/*.g.dart"
''');

Here's the output making that change:

contexts
  /home/test
    packagesFile: /home/test/.dart_tool/package_config.json
    workspace: workspace_0
    analyzedFiles
      /home/test/lib/a.dart
        uri: package:test/a.dart
        analysisOptions_0
        workspacePackage_0_0
      /home/test/lib/b.g.dart
        uri: package:test/b.g.dart
        analysisOptions_0
        workspacePackage_0_0
  /home/test/lib/nested
    packagesFile: /home/test/lib/nested/.dart_tool/package_config.json
    workspace: workspace_1
    analyzedFiles
      /home/test/lib/nested/lib/c.dart
        uri: package:nested/c.dart
        analysisOptions_0
        workspacePackage_1_0
      /home/test/lib/nested/lib/d.g.dart
        uri: package:nested/d.g.dart
        analysisOptions_0
        workspacePackage_1_0
analysisOptions
  analysisOptions_0: /home/test/lib/analysis_options.yaml
workspaces
  workspace_0: PackageConfigWorkspace
    root: /home/test
    pubPackages
      workspacePackage_0_0: PubPackage
        root: /home/test
  workspace_1: PackageConfigWorkspace
    root: /home/test/lib/nested
    pubPackages
      workspacePackage_1_0: PubPackage
        root: /home/test/lib/nested

This is still not quite the layout that you have in your repro but at first blush it does look like it gets at a problem: note that d.g.dart is included in analyzedFiles which it would not if excludes were working as intended.

[gabrielgarciagava]

Yes, 301

Hmm, interesting. In the other hard, it looks like the optionsFile is completely gone from both contexts, which makes b.g.dart to also be included.

I mean, it is not the same as my example since in my example the analyzer:exclude: had effect in the upper directory.

[gabrielgarciagava]

Ahh, I see, the reason for b.d.dart to be included is because my rule is to exclude **/*g.dart, if I add the files one level deeper, then it mimicks my example. I'm running the test locally.

Changed to

    var workspaceRootPath = '/home';
    var testPackageRootPath = '$workspaceRootPath/test';
    var testPackageLibPath = '$testPackageRootPath/lib';

    newPubspecYamlFile(testPackageRootPath, r'''
name: test
''');

    newSinglePackageConfigJsonFile(
      packagePath: testPackageRootPath,
      name: 'test',
    );

    newAnalysisOptionsYamlFile(testPackageLibPath, r'''
analyzer:
  exclude:
    - "**/*.g.dart"
''');

    var nestedNoYamlPath = '$testPackageLibPath/nestedNoYaml';
    newFile('$nestedNoYamlPath/a.dart', '');
    newFile('$nestedNoYamlPath/b.g.dart', '');

    var nestedPath = '$testPackageLibPath/nested';
    newFile('$nestedPath/lib/c.dart', '');
    newFile('$nestedPath/lib/d.g.dart', '');

    newSinglePackageConfigJsonFile(
      packagePath: nestedPath,
      name: 'nested',
    );
    newPubspecYamlFile(nestedPath, r'''
name: nested
''');

And I get

  Actual: 'contexts\n'
            '  /home/test\n'
            '    packagesFile: /home/test/.dart_tool/package_config.json\n'
            '    workspace: workspace_0\n'
            '    analyzedFiles\n'
            '      /home/test/lib/nestedNoYaml/a.dart\n'
            '        uri: package:test/nestedNoYaml/a.dart\n'
            '        analysisOptions_0\n'
            '        workspacePackage_0_0\n'
            '  /home/test/lib/nested\n'
            '    packagesFile: /home/test/lib/nested/.dart_tool/package_config.json\n'
            '    workspace: workspace_1\n'
            '    analyzedFiles\n'
            '      /home/test/lib/nested/lib/c.dart\n'
            '        uri: package:nested/c.dart\n'
            '        analysisOptions_0\n'
            '        workspacePackage_1_0\n'
            '      /home/test/lib/nested/lib/d.g.dart\n'
            '        uri: package:nested/d.g.dart\n'
            '        analysisOptions_0\n'
            '        workspacePackage_1_0\n'
            'analysisOptions\n'
            '  analysisOptions_0: /home/test/lib/analysis_options.yaml\n'
            'workspaces\n'
            '  workspace_0: PackageConfigWorkspace\n'
            '    root: /home/test\n'
            '    pubPackages\n'
            '      workspacePackage_0_0: PubPackage\n'
            '        root: /home/test\n'
            '  workspace_1: PackageConfigWorkspace\n'
            '    root: /home/test/lib/nested\n'
            '    pubPackages\n'
            '      workspacePackage_1_0: PubPackage\n'
            '        root: /home/test/lib/nested\n'
            ''

And as you can see, the only difference between the 2 folders (nested and nestedNoYaml), is that the "buggy" one contains a pubspec.yaml file.

[pq]

And as you can see, the only difference between the 2 folders (nested and nestedNoYaml), is that the "buggy" one contains a pubspec.yaml file.

Yeah. That's a good catch!

@keertip: maybe we can take a look together when you're back?

[keertip]

Thanks for the input on the test @gabrielgarciagava . Can see the failure, now to figure out why.

[gabrielgarciagava]

@keertip @pq This fix was removed in the stable release 3.27. So we cannot upgrade to latest flutter stable once again. We are still stuck on 3.22 :(

I can see that this commit is part of the history, but the code was later removed. The test test_multiplePackageConfigWorkspace_singleAnalysisOptions_exclude was also removed.

[mraleph]

The fix was commited after 3.6 branch was cut. I don't think it was ever cherry picked into the stable. At least I can't find any associated CP requests. We should probably do that. @keertip @pq could you file a CP request? the change looks small enough for that.

[gabrielgarciagava]

@mraleph I thought I saw the commit in the history. Maybe I got confused. But for example, I can see the entire test_multiplePackageConfigWorkspace_singleAnalysisOptions_exclude is not there anymore, while the commit linked here added just some new lines to this test. So I have the impression some later commit actually removed it (or maybe some issue with conflict resolution)

[gabrielgarciagava]

Ahh the other part of the test is in this commit: f4a09ee

I guess this also needs to be cherry picked then.

[pq]

@keertip: do you want to do a CP? If not, let me know and I'll pick it up.

Sorry for the hassle @gabrielgarciagava!

[mraleph]

@pq @keertip any updates on this?

[pq]

Working on a CP here: https://dart-review.googlesource.com/c/sdk/+/403203