Change analysis server's handling of package vs file URI's, and add audit rules.

[stereotype441]

(Capturing some discussions that happened way back in September, and got re-hashed this morning, so that we don't forget how we want to fix this issue)

According to the language spec, a file reached via a "package:" URI is distinct from a file reached via a file URI, even if the two files are in the same location in the filesystem. As a result, if users aren't careful with their choice of URI's in import, export, and part directives, they can wind up in a situation where a file is imprted twice, and thus there are two distinct copies of all of its classes, static functions, and static variables, with some files referring to one copy and other files referring to another. In addition to wasting memory, this can cause some very confusing errors and warnings (e.g. "'Foo' is not assignable to 'Foo'", where the two 'Foo's refer to the two distinct copies of the class 'Foo').

When using the VM and Dart2js, users can avoid this problem by being careful to use package vs file imports in a consistent manner, so that for each library L, the possible import paths from the library containing "main()" to library L either all use file imports or all use package imports*. However, when analysis server is in use, things are not so simple, because analysis considers every file to be a possible starting point for analysis (so that unreferenced code is still analyzed), and it handles each analysis starting point as though it was reached via a file URI. As a result, the analyzer can sometimes produce errors and warnings that would never occur when the code is run.

(*Subtlety: a relative import does not change the type of the URI, so if a library is reached via a relative path, its URI is the same type as that of the file doing the importing.)

The current plan to address this situation is as follows:

1. When considering a file as a starting point for analysis, analyzer should consider files inside "lib" to be reached via a "package" URI rather than a "file" URI, since this is how they will be reached in practice when the package is used.

2. In the fixup that automatically adds "import" statements, use the following algorithm:

   2a. If going between two files inside "lib", use a relative import.

   2b. If going from a file outside "lib" to a file inside "lib", use a "package:" import.

   2c. Otherwise use a relative import (since there is no other choice).

3. Add a set of audit rules that generate hints if the user ever:

   3a. Uses an import/export/part declaration to go from a file inside "lib" to a file outside "lib" (this is unsafe because it would make the package un-runnable once published).

   3b. Uses a relative URI to go from a file outside "lib" to a file inside "lib" (this is unsafe because there could be another way to reach the same file using a package URI)

   3c. Implements a main() function in a file inside "lib" (this is unsafe because the VM and Dart2js always consider the file containing the entry point to be reached via a file URI, but we are trying to ensure that all files inside "lib" are reached via a package URI).

Note that if all of these rules are followed, then files inside "lib" will always be reached via package URI's and files outside "lib" will always be reached via file URI's, regardless of whether the file is being analyzed by the analysis server or by the VM or dart2js.

Note that 3a, 3b, and 3c should all be hints, not errors or warnings, because it is possible for a careful user to write code that violates these rules but still runs fine (and has no errors or warnings according to the spec). For example, it's conceivable that a .dart file inside "lib" might be meant to be used as an asset (i.e. a string of data) and not run directly.

Note also that analyzer currently has some unfinished (and unused) code which attempts to implement audit rules similar to those in 3a, 3b, and 3c (this code was written before we decided on rules 3a, 3b, and 3c). See the PubVerifier class.

(@sigmundch: FYI, I did a minor edit to make the ordered list render correctly in the github UI)

[bwilkerson]

Removed Priority-Unassigned label.
Added Priority-Medium label.

[danrubel]

Old related bug https://code.google.com/p/dart/issues/detail?id=9734

[stereotype441]

Note that the only fix we need to do to stop bogus warnings from occurring is step 1 (When considering a file as a starting point for analysis, analyzer should consider files inside "lib" to be reached via a "package" URI rather than a "file" URI, since this is how they will be reached in practice when the package is used.) Once we've done that, code that makes reasonable choices about whether to use "package:" versus file imports won't generate any bogus warnings. The remaining parts of the plan are to ensure that (a) auto-inserted imports make reasonable choices about what kinds of imports to use, and (b) if the user's code makes poor choices about what kinds of imports to use, we will warn them.

[bwilkerson]

The first step, using package: URI's for files within the lib directory has been implemented in https://codereview.chromium.org/914373004/.

[srawlins]

As we've since added a lot of language tests and much real world usage of various types of URIs, schemas, etc. I imagine this is fixed. Please re-open if not.