double-slash in import doesn't give analysis errors, but fails at compile/run time

[DanTup]

Given the following files:

file1.dart

dantup-macbookpro:foo dantup$ cat file1.dart
import 'folder//file2.dart';

main() {
  a();
}

folder/file2.dart

import '../file3.dart';

a() {
  b();
}

file3.dart

b() {
  print('Hello!');
}

There are no analysis warnings about the double-slash in file1.dart1, however trying to run the file results in an error for the import inside file2.dart. It seems like the relative path starting ../ is not based from the correct folder when there are two slashes.

bin/foo/folder//file2.dart:1:8: Error: Error when reading 'bin/foo/folder/file3.dart': No such file or directory
import '../file3.dart';
       ^
bin/foo/folder//file2.dart:4:3: Error: Method not found: 'b'.
  b();
  ^

It seems like either the analysis server should also complain, or the code should compile/run successfully.

[bwilkerson]

@kmillikin

[kmillikin]

@jensjoha could you take a look at this?

[lrhn]

This is probably correct behavior. URIs are not file paths, and mapping URIs to file paths is not a one-to-one operation.

The URI file:///something/something/folder//file2.dart is different from file:///something/something/folder/file2.dart, both structurally and wrt. operations.
If you resolve ../file3 relative to the former, you get file:///something/something/folder/file3.dart. That's just how the URI reference resolution algorithm is specified.

However, the file system likely treats // the same as /, so the import of file:///something/something/folder//file2.dart works, it's only the file:///something/something/folder/file3.dart that doesn't exist.

If the analyzer is doing resolution at the file system level instead, it might not recognize the problem.

Maybe we should detect // in file paths and warn about them, since they are most likely to be mistakes.

[jensjoha]

My vote would also be that this is correct behavior on the CFE side...
From the spec

An import specifies a URI x where the declaration of an imported library is
to be found.

So the import specifies a uri. That means that we have these URIs:

• file1.dart
• folder//file2.dart
• folder//../file3.dart => folder/file3.dart

where the last one doesn't resolve to anything on the file system.

[DanTup]

Sounds like a bit of a minefield :( Warning on // in imports may be simplest, though I don't know if it's entirely accurate.

I've seen this a few times now which is what led me to raise this. The most recent was Dart-Code/Dart-Code#450 (comment) where hot reload seemed to just not reload any libraries (possibly the // in the path is resulting in the modified file to not match what was loaded?).

[kmillikin]

@lrhn Do you agree that the analyzer should signal that ../file3.dart doesn't exist, or is there some other fix needed here?

[kmillikin]

/cc @stereotype441

[lrhn]

Yes, the code asks for baseUri/folder/file3.dart. If there is no such file, it should be an error.
The code does not ask for 'baseUri/file3.dart', so that should not be what it gets.

The fact that folder//file2.dart works is annoying, but unavoidable. That file does exist according to the file system. From Dart's perspective, it is a file entirely unrelated to folder/file2.dart, so if you import both, you get two different libraries (at least as currently written, we could soften that up and let the code which actually understand the platform's file system determine when two paths represent the same file).

[DanTup]

I was confused why the analyzer didn't hit this, but discovered that if you add export '../file3.dart'; into file2.dart in the sample above and then try to call b() from file1, you'll see the same issue at analysis time:

import 'folder//file2.dart';

main() {
  a();
  b(); // the function b isn't defined
}

But without the double-slash, of course it's fine:

import 'folder/file2.dart';

main() {
  a();
  b(); // this is fine
}

This suggests the issue might be hit (but silent) during analysis. When analyzing file1, the server must have followed the path to folder//file2.dart, then tried (but failed) to resolve its export (hence it didn't find b). I guess the failure is silent because normally if there was a bad import/export in a file, it'd be reported as part of analyzing that file itself (though this is not the case here, since the files contents were discovered through something that is not its real path).

Btw, is it ever valid to have // in an import that would legitimately be different to /? Can you import anything that is not backed by the file system (or a built-in lib)? Like can you import absolute HTTP URIs? If not, isn't an outright ban on // simplest and reasonable?