Analyzer CLI no longer detects duplicate parts

[whesse]

The co19 test Language/Libraries_and_Scripts/Parts/compilation_t02 now fails on the analyzer, because the command-line analyzer no longer detects that the same part is included twice after cache dependencies are no longer checked, a change introduced in
https://codereview.chromium.org/2058253002

Is this intended?

@scheglov @bwilkerson

[whesse]

The test Language/Libraries_and_Scripts/Parts/compilation_t01 also fails. It no longer detects that the same function is declared in both the part and the main file.

[bwilkerson]

No, it isn't intended.

[scheglov]

That's strange.
I'm pretty sure I ran all the tests before committing the change.

And these tests pass even now, after I rollback the status suppression.

scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk ((093672f...))$ d-rat
Please set JAVA_HOME to /Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home
xcodebuild -project dart.xcodeproj -target packages -configuration ReleaseIA32 SYMROOT=/Users/scheglov/Source/Dart/sdk.git/sdk/xcodebuild
Build settings from command line:
    SYMROOT = /Users/scheglov/Source/Dart/sdk.git/sdk/xcodebuild

** BUILD SUCCEEDED **

Test configuration: dart2analyzer_none_release_ia32
Total: 17649 tests
 * 122 tests will be skipped (18 skipped by design)
 * 35 tests are expected to be flaky but not crash
 * 0 tests are expected to flaky crash
 * 16427 tests are expected to pass
 * 29 tests are expected to fail that we won't fix
 * 1022 tests are expected to fail that we should fix
 * 2 tests are expected to crash that we should fix
 * 0 tests are allowed to timeout
 * 0 tests are skipped on browsers due to compile-time error
 * 12 could not be categorized or are in multiple categories

[02:07 | 100% | +17527 | -    0]

They also work in manual testing:

scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk/tests/co19/src ((3f0a4bc...))$
scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts ((3f0a4bc...))$ d-ra compilation_t01.dart
Analyzing [compilation_t01.dart]...
[error] The name 'foo' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 12, col 7)
1 error found.

scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts ((3f0a4bc...))$ d-ra compilation_t02.dart
Analyzing [compilation_t02.dart]...
[error] The name 'Annot' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 8, col 7)
[error] The name 'foo' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 12, col 7)
[error] The name 'bar' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 13, col 5)
3 errors found.

[whesse]

Yes, these are being flakey on the buildbot. So there may be some race condition, or timing, that affects this. I'll think about ways to reproduce this. This is on linux, btw, so maybe it doesn't happen on mac.

[scheglov]

As I can see, the specification does not disallow referring to the same part twice. We started discussing this, but there was no resolution, and the specification has not been changes yet.

It seems like a right thing to do.
Who is in charge of the specification these days? @kevmoo @munificent @lrhn ?

[bwilkerson]

The language team is in charge of the specification, and they are already considering changing parts to use URI's rather than names, which will resolve this issue.

[scheglov]

Will it? This issue is not about part of, it's about part "my_part.dart" twice in the same library.

[bwilkerson]

Oh, sorry, I didn't read carefully enough. No, this is unrelated. I'll send an e-mail to the language team.

[lrhn]

There are basically three solutions:

• Disallow it.
• Allow it and let every part declaration become malformed due to duplicate declaration.
• Allow it and if a part if included twice, it's only used once anyway.

The third is what we do for libraries - if you import the same library twice, it works because both imported names refer to the same declaration. I think we can do something similar for part inclusions.
Like for libraries, it means that you must always fetch each URI at most once. If a library has the same part twice and it fetches it twice from a server that sends different files, then everything is a mess.

The second is what the current specification give us. If you include the contents of the part file twice, it means that its declarations are declared twice in the same scope. Well, maybe, it's not precisely specified what

The top-level declarations at that URI are then compiled by the Dart compiler in the scope of
the current library

really means, but it would not be an incorrect interpretation.

Then again, no real program would be affected by just disallowing it, so that might just be easier.

[bwilkerson]

I would be in favor of disallowing it.

[stereotype441]

Given @lrhn's comment, I'm reclassifying this as a language issue.