@@ -712,51 +712,94 @@ func (s *snapshot) packageHandlesForFile(ctx context.Context, uri span.URI, mode
712
712
return phs , nil
713
713
}
714
714
715
+ // getOrLoadIDsForURI returns package IDs associated with the file uri. If no
716
+ // such packages exist or if they are known to be stale, it reloads the file.
717
+ //
718
+ // If experimentalUseInvalidMetadata is set, this function may return package
719
+ // IDs with invalid metadata.
715
720
func (s * snapshot ) getOrLoadIDsForURI (ctx context.Context , uri span.URI ) ([]PackageID , error ) {
721
+ useInvalidMetadata := s .useInvalidMetadata ()
722
+
716
723
s .mu .Lock ()
724
+
725
+ // Start with the set of package associations derived from the last load.
717
726
ids := s .meta .ids [uri ]
718
- reload := len (ids ) == 0
727
+
728
+ hasValidID := false // whether we have any valid package metadata containing uri
729
+ shouldLoad := false // whether any packages containing uri are marked 'shouldLoad'
719
730
for _ , id := range ids {
720
- // If the file is part of a package that needs reloading, reload it now to
721
- // improve our responsiveness.
731
+ // TODO(rfindley): remove the defensiveness here. s.meta.metadata[id] must
732
+ // exist.
733
+ if m , ok := s .meta .metadata [id ]; ok && m .Valid {
734
+ hasValidID = true
735
+ }
722
736
if len (s .shouldLoad [id ]) > 0 {
723
- reload = true
724
- break
737
+ shouldLoad = true
725
738
}
726
- // TODO(golang/go#36918): Previously, we would reload any package with
727
- // missing dependencies. This is expensive and results in too many
728
- // calls to packages.Load. Determine what we should do instead.
729
739
}
740
+
741
+ // Check if uri is known to be unloadable.
742
+ //
743
+ // TODO(rfindley): shouldn't we also mark uri as unloadable if the load below
744
+ // fails? Otherwise we endlessly load files with no packages.
745
+ _ , unloadable := s .unloadableFiles [uri ]
746
+
730
747
s .mu .Unlock ()
731
748
732
- if reload {
733
- scope := fileURI (uri )
749
+ // Special case: if experimentalUseInvalidMetadata is set and we have any
750
+ // ids, just return them.
751
+ //
752
+ // This is arguably wrong: if the metadata is invalid we should try reloading
753
+ // it. However, this was the pre-existing behavior, and
754
+ // experimentalUseInvalidMetadata will be removed in a future release.
755
+ if ! shouldLoad && useInvalidMetadata && len (ids ) > 0 {
756
+ return ids , nil
757
+ }
758
+
759
+ // Reload if loading is likely to improve the package associations for uri:
760
+ // - uri is not contained in any valid packages
761
+ // - ...or one of the packages containing uri is marked 'shouldLoad'
762
+ // - ...but uri is not unloadable
763
+ if (shouldLoad || ! hasValidID ) && ! unloadable {
764
+ scope := fileLoadScope (uri )
734
765
err := s .load (ctx , false , scope )
735
766
736
- // As in reloadWorkspace, we must clear scopes after loading .
767
+ // Guard against failed loads due to context cancellation .
737
768
//
738
- // TODO(rfindley): simply call reloadWorkspace here, first, to avoid this
739
- // duplication .
740
- if ! errors . Is ( err , context . Canceled ) {
741
- s . clearShouldLoad ( scope )
769
+ // Return the context error here as the current operation is no longer
770
+ // valid .
771
+ if ctxErr := ctx . Err (); ctxErr != nil {
772
+ return nil , ctxErr
742
773
}
743
774
744
- // TODO(rfindley): this doesn't look right. If we don't reload, we use
745
- // invalid metadata anyway, but if we DO reload and it fails, we don't?
746
- if ! s .useInvalidMetadata () && err != nil {
747
- return nil , err
748
- }
775
+ // We must clear scopes after loading.
776
+ //
777
+ // TODO(rfindley): unlike reloadWorkspace, this is simply marking loaded
778
+ // packages as loaded. We could do this from snapshot.load and avoid
779
+ // raciness.
780
+ s .clearShouldLoad (scope )
749
781
750
- s .mu .Lock ()
751
- ids = s .meta .ids [uri ]
752
- s .mu .Unlock ()
782
+ // Don't return an error here, as we may still return stale IDs.
783
+ // Furthermore, the result of getOrLoadIDsForURI should be consistent upon
784
+ // subsequent calls, even if the file is marked as unloadable.
785
+ if err != nil && ! errors .Is (err , errNoPackages ) {
786
+ event .Error (ctx , "getOrLoadIDsForURI" , err )
787
+ }
788
+ }
753
789
754
- // We've tried to reload and there are still no known IDs for the URI.
755
- // Return the load error, if there was one.
756
- if len (ids ) == 0 {
757
- return nil , err
790
+ s .mu .Lock ()
791
+ ids = s .meta .ids [uri ]
792
+ if ! useInvalidMetadata {
793
+ var validIDs []PackageID
794
+ for _ , id := range ids {
795
+ // TODO(rfindley): remove the defensiveness here as well.
796
+ if m , ok := s .meta .metadata [id ]; ok && m .Valid {
797
+ validIDs = append (validIDs , id )
798
+ }
758
799
}
800
+ ids = validIDs
759
801
}
802
+ s .mu .Unlock ()
760
803
761
804
return ids , nil
762
805
}
@@ -1206,25 +1249,26 @@ func (s *snapshot) getMetadata(id PackageID) *KnownMetadata {
1206
1249
1207
1250
// clearShouldLoad clears package IDs that no longer need to be reloaded after
1208
1251
// scopes has been loaded.
1209
- func (s * snapshot ) clearShouldLoad (scopes ... interface {} ) {
1252
+ func (s * snapshot ) clearShouldLoad (scopes ... loadScope ) {
1210
1253
s .mu .Lock ()
1211
1254
defer s .mu .Unlock ()
1212
1255
1213
1256
for _ , scope := range scopes {
1214
1257
switch scope := scope .(type ) {
1215
- case PackagePath :
1258
+ case packageLoadScope :
1259
+ scopePath := PackagePath (scope )
1216
1260
var toDelete []PackageID
1217
1261
for id , pkgPaths := range s .shouldLoad {
1218
1262
for _ , pkgPath := range pkgPaths {
1219
- if pkgPath == scope {
1263
+ if pkgPath == scopePath {
1220
1264
toDelete = append (toDelete , id )
1221
1265
}
1222
1266
}
1223
1267
}
1224
1268
for _ , id := range toDelete {
1225
1269
delete (s .shouldLoad , id )
1226
1270
}
1227
- case fileURI :
1271
+ case fileLoadScope :
1228
1272
uri := span .URI (scope )
1229
1273
ids := s .meta .ids [uri ]
1230
1274
for _ , id := range ids {
@@ -1481,7 +1525,7 @@ func (s *snapshot) AwaitInitialized(ctx context.Context) {
1481
1525
1482
1526
// reloadWorkspace reloads the metadata for all invalidated workspace packages.
1483
1527
func (s * snapshot ) reloadWorkspace (ctx context.Context ) error {
1484
- var scopes []interface {}
1528
+ var scopes []loadScope
1485
1529
var seen map [PackagePath ]bool
1486
1530
s .mu .Lock ()
1487
1531
for _ , pkgPaths := range s .shouldLoad {
@@ -1493,7 +1537,7 @@ func (s *snapshot) reloadWorkspace(ctx context.Context) error {
1493
1537
continue
1494
1538
}
1495
1539
seen [pkgPath ] = true
1496
- scopes = append (scopes , pkgPath )
1540
+ scopes = append (scopes , packageLoadScope ( pkgPath ) )
1497
1541
}
1498
1542
}
1499
1543
s .mu .Unlock ()
@@ -1505,7 +1549,7 @@ func (s *snapshot) reloadWorkspace(ctx context.Context) error {
1505
1549
// If the view's build configuration is invalid, we cannot reload by
1506
1550
// package path. Just reload the directory instead.
1507
1551
if ! s .ValidBuildConfiguration () {
1508
- scopes = []interface {} {viewLoadScope ("LOAD_INVALID_VIEW" )}
1552
+ scopes = []loadScope {viewLoadScope ("LOAD_INVALID_VIEW" )}
1509
1553
}
1510
1554
1511
1555
err := s .load (ctx , false , scopes ... )
@@ -1527,7 +1571,7 @@ func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
1527
1571
files := s .orphanedFiles ()
1528
1572
1529
1573
// Files without a valid package declaration can't be loaded. Don't try.
1530
- var scopes []interface {}
1574
+ var scopes []loadScope
1531
1575
for _ , file := range files {
1532
1576
pgf , err := s .ParseGo (ctx , file , source .ParseHeader )
1533
1577
if err != nil {
@@ -1536,7 +1580,8 @@ func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
1536
1580
if ! pgf .File .Package .IsValid () {
1537
1581
continue
1538
1582
}
1539
- scopes = append (scopes , fileURI (file .URI ()))
1583
+
1584
+ scopes = append (scopes , fileLoadScope (file .URI ()))
1540
1585
}
1541
1586
1542
1587
if len (scopes ) == 0 {
@@ -1560,7 +1605,7 @@ func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
1560
1605
event .Error (ctx , "reloadOrphanedFiles: failed to load" , err , tag .Query .Of (scopes ))
1561
1606
s .mu .Lock ()
1562
1607
for _ , scope := range scopes {
1563
- uri := span .URI (scope .(fileURI ))
1608
+ uri := span .URI (scope .(fileLoadScope ))
1564
1609
if s .noValidMetadataForURILocked (uri ) {
1565
1610
s .unloadableFiles [uri ] = struct {}{}
1566
1611
}
0 commit comments