Warn when glob patterns in the analysis options file start with a slash

[dmrickey]

Per the glob package documentation, glob patterns should start with leading slashes, otherwise root files won't be found by a glob pattern. https://pub.dev/packages/glob#:~:text=If%20**%20appears,by%20it%20either. Starting with a leading slash does not work on windows--the pattern simply appears to not be recognized.

I added this bit of code to main.yaml [1, 2, 3].forEach((i) => print(i)); and this rule to the analysis options

analyzer:
  exclude:
    - "/**/main.dart" # this doesn't work on windows

This works fine on my coworkers macs

File is not excluded

Issue is related to:

• Analyzer

If you aren't sure, file the issue here and we'll find the right home for it. In your issue, please include:

• Dart SDK Version (dart --version)

  Dart SDK version: 3.0.3 (stable) (Wed May 31 15:35:05 2023 +0000) on "windows_x64"

• Whether you are using Windows, MacOSX, or Linux (if applicable)

  I'm on windows.

• Whether you are using Chrome, Safari, Firefox, Edge (if applicable)

  n/a

Here is a repo that shows the problem in main.dart and analysis options. But it should not be necessary as it's simply a brand new project with a simple change in analysis options, and one line of code introduced to show that it is being analyzed despite being excluded. https://github.com/dmrickey/dart-glob-test

[scheglov]

I suspect that we should use relative URIs. And you have such relative URI. Why don't you use it?

[DanTup]

**/foo.dart won't match foo.dart in the root folder (presumably because it's checking the relative path foo.dart and not matching a /), though I'm not sure that's all that important since it's probably rare to have Dart files in the root.

The behaviour should probably be consistent between Windows/non-Windows though?

"{**/foo.dart,foo.dart}" does handle both root/non-root on Windows, although it seems like "{**/,}foo.dart" terminates the server - I'll look into why.. (Edit: opened https://github.com/dart-lang/glob/issues/80)

[DanTup]

Maybe related to the original issue? https://github.com/dart-lang/glob/issues/9

[dmrickey]

I suspect that we should use relative URIs. And you have such relative URI. Why don't you use it?

If you're asking why I'm not using **/main.dart and leaving it commented out, that's because the linked documentation basically says that glob patterns should start with a / in dart's implementation of the glob pattern to ensure it matches all expected files.

If that's not what you're asking then I'm not sure what you mean.

[bwilkerson]

The documentation for the excludes list in the analysis options file isn't clear, but these globs are interpreted to be relative to the directory containing the analysis options files, and hence should never begin with a slash. I've opened a request to have the docs updated.

Assuming that relative globs work on Windows, I believe the feature is working as intended.

[dmrickey]

@bwilkerson

There's still an issue and this should not be closed. Regardless of how the docs were misinterpreted, the behavior should be the same between windows/unix--which it currently isn't.

[bwilkerson]

Ok. I'm unclear as to what the expected behavior is when a glob starts with a / but is being used relative to a non-root directory. What behavior are you proposing?

[dmrickey]

Originally we had our glob patterns without the leading slash. At some point we introduced a lot of new lint rules, and as part of that someone saw the documentation I originally linked and said "hey, dart is special and our glob patterns are apparently wrong. Supposedly they're supposed to start with a leading slash to catch files we may have been missing." That dev was on a mac and when they made that change, the glob pattern still worked so it was merged in. I'm on windows and later realized that the leading slash was causing problems and lead me here.

The short of it, I'm not saying I have an answer on what a leading slash should mean, I'm just saying the behavior should be consistent and it should behave the same way in both environments. Ideally that behavior would be whatever is reflected in the docs.

[DanTup]

I think the issue here is in package:glob rather than the analyzer:

Windows

**/main.dart  main.dart                                   false
**/main.dart  lib\main.dart                               true
**/main.dart  C:\Dev\Test Projects\globtest\main.dart     false
**/main.dart  C:\Dev\Test Projects\globtest\lib\main.dart true
/**/main.dart main.dart                                   false
/**/main.dart lib\main.dart                               false
/**/main.dart C:\Dev\Test Projects\globtest\main.dart     false
/**/main.dart C:\Dev\Test Projects\globtest\lib\main.dart false

Mac

**/main.dart  main.dart                                                false
**/main.dart  lib/main.dart                                            true
**/main.dart  /Users/danny/Desktop/dart_samples/globtest/main.dart     false
**/main.dart  /Users/danny/Desktop/dart_samples/globtest/lib/main.dart true
/**/main.dart main.dart                                                true
/**/main.dart lib/main.dart                                            true
/**/main.dart /Users/danny/Desktop/dart_samples/globtest/main.dart     true
/**/main.dart /Users/danny/Desktop/dart_samples/globtest/lib/main.dart true

Although if the server always uses relative paths, maybe it's also reasonable to generate a diagnostic for any glob starting "/" saying relative globs should be used? (If I understand correctly, @dmrickey can remove all the leading slashes and things will work as expected on all platforms?).

[bwilkerson]

Yes, if the only difference between the way globs work on different platforms is when using absolute paths (starting with a slash), then the fix is to always us relative paths. It would make sense for us to issue a warning to that effect when we find a glob that starts with a slash in that file in order to help users spot the problem.

[dmrickey]

(If I understand correctly, @dmrickey can remove all the leading slashes and things will work as expected on all platforms?).

Yes that's correct. Everything seems to work as intended when I remove the leading /