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.24k stars 1.57k forks source link

Using dart analyze with conditional imports/exports (web support) #56748

Open loryruta opened 1 month ago

loryruta commented 1 month ago

I'm sorry if this issue is duplicated, but I couldn't find anything similar around.

Background

I have a package for which I'm providing native and web support that has the following structure:

native/
    cat.dart
    dog.dart
web/
    cat.dart
    dog.dart
cat.dart
dog.dart
animal_utils.dart

Top-level .dart files are:

/* ./cat.dart */
export 'native/cat.dart' if (dart.library.html) 'web/cat.dart';

/* ./dog.dart */
export 'native/dog.dart' if (dart.library.html) 'web/dog.dart';

/* ./animal_utils.dart */
import './cat.dart'; // Resolved either to native/cat.dart or web/cat.dart depending on the platform (*)
import './dog.dart'; // Resolved either to native/dog.dart or web/dog.dart depending on the platform (*)

Cat getNearbyCat(Dog dog) { /* ... */ }

While native/ and web/ folder contain Cat and Dog classes. Cat and Dog have the same API but different implementations in native/ and web/ (either calling ffi or js_interop bindings).

The issue

web/dog.dart contains the following code:

/* web/dog.dart */
import '../animal_utils.dart';

class Dog {
    void bark() {
         Cat cat = animal_utils.getNearbyCat(this);
    }
}

This code will work fine when building and running for web, because all conditional exports are correctly resolved to web (i.e. dart.library.html is present). But when using dart analyze all conditional exports are resolved to native. Therefore getNearbyCat will expect a native/dog.dart instead of web/dog.dart leading to an analyzer error.

Is there any option that I'm missing? My project structure maybe isn't correct? Is it a known issue?

Thank you,

P.S.: I'm currently excluding the web/ folder in analysis_options.yaml. In this way the dart analyzer will just analyze native files and everything works fine. But it looks unavoidable when using pana.

dart-github-bot commented 1 month ago

Summary: The user is experiencing an issue where dart analyze incorrectly resolves conditional imports/exports to the native platform when analyzing web code. This leads to errors because the web code expects the web implementation of classes, but the analyzer is using the native implementation.

lrhn commented 1 month ago

There is nothing you are missing. Static source code analysis is platform agnostic (or rather, platform oblivious), and the analyzer will assume all conditional imports to have failing tests, and always import the default library. That should work for any full program, because presumably the test being false is an option.

The problem is that the analyzer analyzes individual files, not complete programs, and the libraries that are not conditionally imported are still analyzed by themselves, even if no conditional import would choose them.

That becomes an issue in exactly cases like this, where a general file has a conditional import, so that the general, uncoditional, file's member's signatures depend on the platform, and then platform specific code depends on the general code. That is, where a library that is intended to be imported conditionally depends on another library that is conditionally imported.

The web/dog.dart code is always analyzed, because it's a library which can be analyzed by itself. The library is not itself conditional on the web platform, even though it really should only be part of a program when the platform is web. So the analyzer analyzes it, sees an import if animal_utils.dart which then has a conditional limport of native/dog.dart, and predictably things break down.

The only "solution" I have found to this, is to stratify the platform specific code, so that you never have platform specific code depending on general code that depends on platform specific code. (Where "platform specific code" is code imported through a conditional import or export.)

Said differently, a library intended to be imported conditionally must never depend on another library through a conditional import/export, at least not on the same condition that would be used to choose or not choose the former library.

In this case, it would probably mean that you need a getNearbyCat for each platform, because its signature (or implementation for that matter) depends on the selected platform, and it's being used from platform specific code which is analyzed independently of the platform.

When Dart gets conditional part directives, it may become easier to avoid code for different platforms being analyzed independently, but it will also make some parts never be analyzed, so there is a trade-off there.

loryruta commented 1 month ago

In this case, it would probably mean that you need a getNearbyCat for each platform, because its signature (or implementation for that matter) depends on the selected platform, and it's being used from platform specific code which is analyzed independently of the platform.

But this leads to duplicated code., doesn't it? Mine was just a minimal example, actually I have a large codebase that is shared and little platform -depedent things. Copying 'common' code (like getNearbyCat) for both platforms is a thing I'd like to avoid...

lrhn commented 1 month ago

But this leads to duplicated code, doesn't it?

Quite likely, yes.

Philosohically, you can question whether it's duplicate code, since it's not the same code. The two versions refer to completely different classes, and there exists code that refer to either of the versions. They are different classes that just happen to share code shape. It's like saying that

int length(String string) => string.length;

and

int length(List<Object?> list) => list.length;

are duplicated code, but they can't shared the same code because list and String has no shared supertype that has a length getter. It's not the same length they're talking about, so the it's not duplicate code.

Here you have code that works on two different Dogs, and code that refers to (or tries to refer to) both versions of that code. Even if a full program can only contain one of these, the project contains both. But the analyzer can only see one version, so the code that depends on the other version is not consistent.

eernstg commented 1 month ago

In response to the topic 'Using dart analyze with conditional imports/exports', we have to recognize that the analyzer does not support this scenario. However, you can do a number of things.

As @lrhn mentioned, the analyzer will use the non-conditional (first) branch of the conditional import directive, and ignore all the conditional branches. There's no reason to assume that the static analysis performed in this manner will produce results that are in any way similar to the results where any other choice is made (that is, your program may be labeled as having 'no issues', but the analysis of the actual choice of branches for a given compilation may produce any number of compile-time errors, or vice versa).

When it comes to compile-time errors, though, you can rely on the common front end. That is, compile the program using dart compile js (or basically anything other than dart analyze), and you will then know whether or not it has any compile-time errors for the specific configuration which was present during that compilation. (You don't know anything about any other configuration.)

If you want to know about warnings and lints then the common front end won't help you. In this case you could edit the code such that all the conditional imports are changed to simple (non-conditional) imports, importing the library that would be imported by the compiler in the configuration of interest. This will give you the true and precise information.

You could also try to make all the different branches "equivalent". That is, if a conditional import may import lib1.dart, lib2.dart, or lib3.dart, then you could try to maintain the property (manually, there is no help from the tools to do this) that all those libraries have exactly the same set of top-level declarations with exactly the same meaning of every single identifier (it isn't enough that they all declare a top-level function MyType foo() {...} if the meaning of MyType is different in the three libraries).

If it will work for you then you could organize your program to make this approach safe: Make sure that if a library is conditionally imported then it doesn't declare any types. It can declare variables and functions, and it can use types in signatures, as long as they are imported from one or more libraries that are common to all the conditionally imported libraries (that is, they must all use the types in these shared libraries, and no other types). This means that they all agree on the meaning of every type name, and that means that it is enough to check that they also agree precisely on which functions and variables they are exporting, and on their signatures.