Analyzer doesn't warn me that I haven't included a dependency

[kasperpeulen]

I have the following dependencies:

So I have not included path, however, I can just include path in my lib files.

I think this is because my dev_dependencies depend on it. I think I should get an error, that I haven't included path in my dependencies. (when I use it in lib directory)

[zoechi]

dup of #57175

[bwilkerson]

This seems like more of an error than a lint. The only reason analyzer doesn't already create errors for this is because we only have one place to look to find the packages to resolve code against, even though pub defines two sets of packages.

@munificent @nex3 Now that we have .packages files, it would theoretically be possible for pub to create one .packages file in the root directory of the project that contains both dependencies and dev_dependencies and a second in 'lib' that would contain only dependencies. We'd need to fix the way analyzer handles this to prevent a performance hit, but it would be accurate. What do you think?

[nex3]

I'm not a big fan of that idea. The .packages file is specifically for executing Dart files, and you don't execute them from lib/. I think users would have a hard time understanding why there were multiples, especially since we're moving away from putting .packages files everywhere. And that still wouldn't accurately handle dependency overrides, which can force a dependency to come from a different source when run locally.

I think a better solution would be for the analyzer to go to the source and look at the pubspec. The format is very simple and stable.

[bwilkerson]

The .packages file is specifically for executing Dart files ...

I disagree. The .packages file is specifically for resolving package: URI's, whether for execution or analysis.

... we're moving away from putting .packages files everywhere.

Did you mean "we're moving away from putting packages directories everywhere.", or is there another issue I wasn't aware of?

And that still wouldn't accurately handle dependency overrides, which can force a dependency to come from a different source when run locally.

I don't understand what problem you're referring to. Could you elaborate?

[nex3]

I disagree. The .packages file is specifically for resolving package: URI's, whether for execution or analysis.

Okay, but the only way that resolution logic is applied is based on an entrypoint. There are no entrypoints in lib, and there's nothing in the .packages spec that says that a .packages file applies to libraries nested beneath it in the filesystem.

Did you mean "we're moving away from putting packages directories everywhere.", or is there another issue I wasn't aware of?

Yes, sorry.

I don't understand what problem you're referring to. Could you elaborate?

If I write

name: myapp
dependencies:
  source_span: ^1.0.0
dependency_overrides:
  source_span: {path: ../source_span}

The .packages entry for the public view of myapp would refer to a completely different location than the local view, because the latter includes dependency overrides and the former does not. In fact, we don't even have a valid referent for the public view, because that requires doing version resolution without the dependency override.

[bwilkerson]

Okay, but the only way that resolution logic is applied is based on an entrypoint.

True, but analyzer has to be a bit liberal in it's definition of "entry point" :-) in order to be useful in cases where there is no actual entry point, such as a reusable library. We take it to mean any Dart file in the package.

... there's nothing in the .packages spec that says that a .packages file applies to libraries nested beneath it in the filesystem.

Seems to me that that's implied by the following portion of the look-up algorithm:

• [If no resolution method has been found in previous steps], then the tool checks for the presence of a .packages file in the parent directory of the entry point, and recursively check each parent directory up to the root directory until a .packages file is found.

... the public view of myapp would refer to a completely different location than the local view ...

True, but analyzer is a development time tool, so it's only interested in the local view, which is what it gets when pub is run in the package containing the dependency override.

[nex3]

If it's only interested in the local view, then why do you want to exclude dev dependencies? They're part of the local view too. Technically, as far as pub is concerned, they're a subset of dependency overrides.

[bwilkerson]

If it's only interested in the local view, then why do you want to exclude dev dependencies?

Sorry, I misspoke. There are more (possible) sets of package resolution information than I have terminology for.

There is a difference between the packages that can validly be referenced inside 'lib' (dependencies) and the packages that can be referenced elsewhere (dependencies + dev dependencies). That defines two different sets of package resolution data and it's important to users so that they can be told if they reference things inside 'lib' that won't be valid when they publish their package.

There is an orthogonal dimension, which is the package resolution data both with and without dependency overrides. (In my mind that makes four sets so far.) I don't think users care as much about this distinction and analyzer should just trust that the sets with dependency overrides are what users really wants their code to be analyzed against. It appears that I was misusing "local" when I used it to mean "with dependency overrides".

(I'm still not sure which combinations you mean by "public" and "local".)

[munificent]

How about we look at this a different way:

Let's say the analyzer could show the user the warnings we're talking about here without adding an extra .packages file in lib. (This could be by parsing the pubspec.yaml, separating out the dev_dependencies in pub list-package-dirs, etc.)

If we did that, and users were getting the warnings they wanted, would there be any additional value for users in having that extra .packages file in there?

My belief is that the answer is no. The file doesn't add end user value. It's just a communication channel between analyzer and pub. Given that, how about we pick a communication channel that doesn't clutter up their file system?

If analyzer still uses list-package-dirs, we could add another key for dev_dependencies to that. Or we could add machine-readable output to pub deps. Or something else.

[bwilkerson]

If analyzer still uses list-package-dirs ...

It does, but only when it cannot find a .packages file or a packages directory, so it wouldn't be a general enough solution.

Is there a package that contains the code pub uses to read pubspec.yaml files? I don't want to duplicate that code, but would be willing to use it.

[nex3]

Is there a package that contains the code pub uses to read pubspec.yaml files? I don't want to duplicate that code, but would be willing to use it.

There's not, although we've considered factoring it out from pub before. It may be overkill in this case, though; the information you'd need is just document["dependencies"].containsKey(packageName). Even with YAML parsing and proper error handling, that should only be a few lines of code.

[bwilkerson]

We really need to do something to differentiate between these two bodies of code, but handling it correctly isn't going to be trivial. Given that it isn't stopping anyone from developing, it probably won't happen soon.

[srawlins]

This is a duplicate of a request in the linter.

[srawlins]

Duplicate of #57175