Skip to content

Commit b93274b

Browse files
committed
gopls/internal/cache: remove overzealous bug.Report
defineView's doc comment confidently states that the only possible failure is a canceled context... yet the very first thing it does is return an error if the folder path is invalid---and it was ever thus since the comment was added in CL 552315. D'oh! Downgrading to a non-bug error. Fixes golang/go#70909 Change-Id: Ic58199786816ad7de9f77d18a7d46795440bdf01 Reviewed-on: https://go-review.googlesource.com/c/tools/+/639435 Reviewed-by: Robert Findley <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 960f0f4 commit b93274b

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

gopls/internal/cache/session.go

+5-6
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ func (s *Session) View(id string) (*View, error) {
348348
// SnapshotOf returns a Snapshot corresponding to the given URI.
349349
//
350350
// In the case where the file can be can be associated with a View by
351-
// bestViewForURI (based on directory information alone, without package
351+
// [RelevantViews] (based on directory information alone, without package
352352
// metadata), SnapshotOf returns the current Snapshot for that View. Otherwise,
353353
// it awaits loading package metadata and returns a Snapshot for the first View
354354
// containing a real (=not command-line-arguments) package for the file.
@@ -551,13 +551,12 @@ checkFiles:
551551
}
552552
def, err = defineView(ctx, fs, folder, fh)
553553
if err != nil {
554-
// We should never call selectViewDefs with a cancellable context, so
555-
// this should never fail.
556-
return nil, bug.Errorf("failed to define view for open file: %v", err)
554+
// e.g. folder path is invalid?
555+
return nil, fmt.Errorf("failed to define view for open file: %v", err)
557556
}
558557
// It need not strictly be the case that the best view for a file is
559558
// distinct from other views, as the logic of getViewDefinition and
560-
// bestViewForURI does not align perfectly. This is not necessarily a bug:
559+
// [RelevantViews] does not align perfectly. This is not necessarily a bug:
561560
// there may be files for which we can't construct a valid view.
562561
//
563562
// Nevertheless, we should not create redundant views.
@@ -572,7 +571,7 @@ checkFiles:
572571
return defs, nil
573572
}
574573

575-
// The viewDefiner interface allows the bestView algorithm to operate on both
574+
// The viewDefiner interface allows the [RelevantViews] algorithm to operate on both
576575
// Views and viewDefinitions.
577576
type viewDefiner interface{ definition() *viewDefinition }
578577

gopls/internal/cache/view.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -809,9 +809,10 @@ func (s *Session) invalidateViewLocked(ctx context.Context, v *View, changed Sta
809809
// If forURI is non-empty, this view should be the best view including forURI.
810810
// Otherwise, it is the default view for the folder.
811811
//
812-
// defineView only returns an error in the event of context cancellation.
812+
// defineView may return an error if the context is cancelled, or the
813+
// workspace folder path is invalid.
813814
//
814-
// Note: keep this function in sync with bestView.
815+
// Note: keep this function in sync with [RelevantViews].
815816
//
816817
// TODO(rfindley): we should be able to remove the error return, as
817818
// findModules is going away, and all other I/O is memoized.
@@ -838,11 +839,11 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forFile fil
838839
// add those constraints to the viewDefinition's environment.
839840

840841
// Content trimming is nontrivial, so do this outside of the loop below.
841-
// Keep this in sync with bestView.
842+
// Keep this in sync with [RelevantViews].
842843
path := forFile.URI().Path()
843844
if content, err := forFile.Content(); err == nil {
844845
// Note the err == nil condition above: by convention a non-existent file
845-
// does not have any constraints. See the related note in bestView: this
846+
// does not have any constraints. See the related note in [RelevantViews]: this
846847
// choice of behavior shouldn't actually matter. In this case, we should
847848
// only call defineView with Overlays, which always have content.
848849
content = trimContentForPortMatch(content)

0 commit comments

Comments
 (0)