await a missed test failure, fix broken test

[natebosch]

This test should be failing but the error is lost because of the way the
async failure interacts with the runBuilder error handling zone.
dart-lang/test#1670

Use await expectLater instead of await expect to ensure the error is
not lost.

Change the setup for the test to include two imports instead of 1 import
and a local class definition. Move the usage to an annotation so that it
is easy to find from the library using it.

[natebosch]

This test was introduced during the migration to null safety, I'm not 100% sure I understand the value it brings.

@jakemac53 - do you know the intention behind this test?

[jakemac53]

I don't fully understand the test, no. I am not sure why the asset wouldn't be resolvable?

[natebosch]

@simolus3 - do you recall the intent behind this test?

[simolus3]

I'm not sure, but I think it may have been something about MultiplyDefinedElements not having a source? This part from the same commit looks related

@@ -263,12 +272,17 @@ class AnalyzerResolver implements ReleasableResolver {
 
   @override
   Future<AssetId> assetIdForElement(Element element) async {
-    final uri = element.source.uri;
-    if (!uri.isScheme('package') && !uri.isScheme('asset')) {
+    final source = element.source;
+    if (source == null) {
       throw UnresolvableAssetException(
-          '${element.name} in ${element.source.uri}');
+          '${element.name} does not have a source');
+    }
+
+    final uri = source.uri;
+    if (!uri.isScheme('package') && !uri.isScheme('asset')) {
+      throw UnresolvableAssetException('${element.name} in ${source.uri}');
     }
-    return AssetId.resolve('${element.source.uri}');
+    return AssetId.resolve('${source.uri}');
   }
 }

Looks like we'd crash before, I've realized that with the null-safety migration and changed it to throw the exception instead?

[jakemac53]

I'm not sure, but I think it may have been something about MultiplyDefinedElements not having a source? This part from the same commit looks related

That makes sense, but I don't get why this is a MultiplyDefinedElement in this case? It seems to be grabbing it directly from the imported library?

[simolus3]

Yeah looking at the test it seems like I got it wrong. I also couldn't figure out an easy way to get a MultiplyDefinedElement now, so maybe the test should just be removed?

[jakemac53]

I think you could try defining the same class twice in the same library - no need for any imported library?

[simolus3]

I've tried that, but still no luck. With the following library, the analyzer returns the first class for getType('SomeClass') and exportNamespace.get('SomeClass'):

class SomeClass {}
class SomeClass {}

It looks like it's the same when I lookup the element from the public namespace when importing two different libraries both defining SomeClass.

[jakemac53]

Interesting, I suppose we can just go without testing this behavior, but we should leave in the code to throw if we don't have a source.

[natebosch]

I think I found a pattern that can give us an element without a source. @jakemac53 PTAL