From 3b49ecbfa78e18ac2c2abf671cbb623b752a78d8 Mon Sep 17 00:00:00 2001 From: Peng Date: Wed, 11 Sep 2024 22:05:00 -0400 Subject: [PATCH] rules_go improvement to externalize the nogo fix --- go/.DS_Store | Bin 8196 -> 0 bytes go/private/actions/compilepkg.bzl | 1 + go/private/rules/binary.bzl | 2 + go/private/rules/library.bzl | 2 +- go/private/rules/test.bzl | 7 + go/private/sdk.bzl | 1 + go/tools/builders/BUILD.bazel | 15 +- go/tools/builders/builder.go | 2 +- go/tools/builders/nogo.go | 11 +- go/tools/builders/nogo_change.go | 95 ++++---- .../builders/nogo_change_serialization.go | 14 +- .../nogo_change_serialization_test.go | 39 ++++ go/tools/builders/nogo_change_test.go | 214 ++++++++++++++++++ go/tools/builders/nogo_edit.go | 159 ------------- go/tools/builders/nogo_main.go | 38 ++-- go/tools/builders/nogo_validation.go | 2 +- go/tools/builders/stdlib.go | 2 +- 17 files changed, 368 insertions(+), 236 deletions(-) delete mode 100644 go/.DS_Store create mode 100644 go/tools/builders/nogo_change_serialization_test.go create mode 100644 go/tools/builders/nogo_change_test.go delete mode 100644 go/tools/builders/nogo_edit.go diff --git a/go/.DS_Store b/go/.DS_Store deleted file mode 100644 index 3578b490b334802aa5d12038a5bbd91066a9f547..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 8196 zcmeHMyH3L}6upMh1}Y>bU|`730)G%tnV3*FCMe}4rKBp4i6Oth#DZ85D;r;c_y`{V zfe;Jl+HPt$Nf8SIab4LbvCoZ9kDa(qiAb%UwwHFl~DGrDO|Iz_|K7`1Ok;T-Y zesrMFTL54M)jHuA`v4siT8u2F26ZT|X?73FhAO+oP$nGhA=818#nhk)CuPD(*`Af% zp(x!u;zCU)6&aLL91sWc4)ET+N;{O$5$)yo@725E_D<4hwBn?N`rv)@>2l-kaV5`b zf63_{xbF${U0b(0j_HJYv<)>!I6C&~fz{F?3S4Iu(gtS-gS zc>?DWDDL=5=b|Ext1)=UH0G|=Ne1dh@qArNtg8lHJ-BzKwe5PUDlh!t9Adbd&U?@O zG0m|Cipp7@o@&Z^0;oB)^DCy^;Sp_K6meWli&vSZ+_hYvM)5pdq;*YVRDfZ?T?=m@48gX@cOU! z*KoZG^O74OGo}Xlkk5HCG0OVSMo1C|{*VLXM(aH9|JTaj|No(HBvQozap1cSs7j;R zSVfdx)eE9|l@F1(kU23gHK;>Scs~Sip8a8n;}BGii7ciDQG+Be0<;ZMhyy?Bz$Yiw B4LAS* diff --git a/go/private/actions/compilepkg.bzl b/go/private/actions/compilepkg.bzl index e22d44b22..96a22da01 100644 --- a/go/private/actions/compilepkg.bzl +++ b/go/private/actions/compilepkg.bzl @@ -90,6 +90,7 @@ def emit_compilepkg( fail("nogo must be specified if and only if out_nogo_validation is specified") if bool(nogo) != bool(out_nogo_fix): fail("nogo must be specified if and only if out_nogo_fix is specified") + if cover and go.coverdata: archives = archives + [go.coverdata] diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl index 686d4f4ea..f47679dcf 100644 --- a/go/private/rules/binary.bzl +++ b/go/private/rules/binary.bzl @@ -148,12 +148,14 @@ def _go_binary_impl(ctx): executable = executable, ) validation_output = archive.data._validation_output + nogo_fix_output = archive.data._out_nogo_fix providers = [ archive, OutputGroupInfo( cgo_exports = archive.cgo_exports, compilation_outputs = [archive.data.file], + nogo_fix = [nogo_fix_output] if nogo_fix_output else [], _validation = [validation_output] if validation_output else [], ), ] diff --git a/go/private/rules/library.bzl b/go/private/rules/library.bzl index 1a9e5ce15..c9ddda534 100644 --- a/go/private/rules/library.bzl +++ b/go/private/rules/library.bzl @@ -66,7 +66,7 @@ def _go_library_impl(ctx): OutputGroupInfo( cgo_exports = archive.cgo_exports, compilation_outputs = [archive.data.file], - out_nogo_fix = [nogo_fix_output] if nogo_fix_output else [], + nogo_fix = [nogo_fix_output] if nogo_fix_output else [], _validation = [validation_output] if validation_output else [], ), ] diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index 5ee3f855c..f2a3fe7af 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -68,6 +68,7 @@ def _go_test_impl(ctx): ) validation_outputs = [] + nogo_fix_outputs = [] # Compile the library to test with internal white box tests internal_library = go.new_library(go, testfilter = "exclude") @@ -75,6 +76,11 @@ def _go_test_impl(ctx): internal_archive = go.archive(go, internal_source) if internal_archive.data._validation_output: validation_outputs.append(internal_archive.data._validation_output) + if internal_archive.data._out_nogo_fix: + # Note we will not include external_archive.data._out_nogo_fix since + # they correspond to the packages external to the current workspace. + nogo_fix_outputs.append(internal_archive.data._out_nogo_fix) + go_srcs = [src for src in internal_source.srcs if src.extension == "go"] # Compile the library with the external black box tests @@ -199,6 +205,7 @@ def _go_test_impl(ctx): ), OutputGroupInfo( compilation_outputs = [internal_archive.data.file], + nogo_fix = nogo_fix_outputs, _validation = validation_outputs, ), coverage_common.instrumented_files_info( diff --git a/go/private/sdk.bzl b/go/private/sdk.bzl index ad6e1e8d3..486919d7f 100644 --- a/go/private/sdk.bzl +++ b/go/private/sdk.bzl @@ -91,6 +91,7 @@ def _go_download_sdk_impl(ctx): ) data = ctx.read("versions.json") + ctx.delete("versions.json") sdks_by_version = _parse_versions_json(data) if not version: diff --git a/go/tools/builders/BUILD.bazel b/go/tools/builders/BUILD.bazel index 707e41ac9..485b1dd46 100644 --- a/go/tools/builders/BUILD.bazel +++ b/go/tools/builders/BUILD.bazel @@ -31,6 +31,20 @@ go_test( ], ) +go_test( + name = "nogo_change_test", + size = "small", + srcs = [ + "nogo_change.go", + "nogo_change_serialization.go", + "nogo_change_serialization_test.go", + "nogo_change_test.go", + ], + deps = [ + "@org_golang_x_tools//go/analysis", + ], +) + go_test( name = "stdliblist_test", size = "small", @@ -99,7 +113,6 @@ go_source( "flags.go", "nogo_change.go", "nogo_change_serialization.go", - "nogo_edit.go", "nogo_main.go", "nogo_typeparams_go117.go", "nogo_typeparams_go118.go", diff --git a/go/tools/builders/builder.go b/go/tools/builders/builder.go index 26f596eb7..fd0a5ba6a 100644 --- a/go/tools/builders/builder.go +++ b/go/tools/builders/builder.go @@ -62,6 +62,6 @@ func main() { log.SetPrefix(verb + ": ") if err := action(rest); err != nil { - log.Fatalf("\n$$$$$$$$$$$$$$$$$$$$$$$$ fatal: %+v", err) + log.Fatal(err) } } diff --git a/go/tools/builders/nogo.go b/go/tools/builders/nogo.go index cdf48b5ff..1615b1311 100644 --- a/go/tools/builders/nogo.go +++ b/go/tools/builders/nogo.go @@ -40,8 +40,7 @@ func nogo(args []string) error { fs.StringVar(&nogoPath, "nogo", "", "The nogo binary") fs.StringVar(&outFactsPath, "out_facts", "", "The file to emit serialized nogo facts to") fs.StringVar(&outLogPath, "out_log", "", "The file to emit nogo logs into") - - fs.StringVar(&nogoFixPath, "fixpath", "", "The fix path") + fs.StringVar(&nogoFixPath, "fixpath", "", "The path of the file that stores the nogo fixes") if err := fs.Parse(args); err != nil { return err @@ -90,6 +89,7 @@ func nogo(args []string) error { } func runNogo(workDir string, nogoPath string, srcs, ignores []string, facts []archive, packagePath, importcfgPath, outFactsPath string, outLogPath string, nogoFixPath string) error { + if len(srcs) == 0 { // emit_compilepkg expects a nogo facts file, even if it's empty. // We also need to write the validation output log. @@ -101,14 +101,15 @@ func runNogo(workDir string, nogoPath string, srcs, ignores []string, facts []ar if err != nil { return fmt.Errorf("error writing empty nogo log file: %v", err) } + err = os.WriteFile(nogoFixPath, nil, 0o666) + if err != nil { + return fmt.Errorf("error writing empty nogo fix file: %v", err) + } return nil } args := []string{nogoPath} args = append(args, "-p", packagePath) args = append(args, "-fixpath", nogoFixPath) - - - // args = append(args, "-json") args = append(args, "-importcfg", importcfgPath) for _, fact := range facts { args = append(args, "-fact", fmt.Sprintf("%s=%s", fact.importPath, fact.file)) diff --git a/go/tools/builders/nogo_change.go b/go/tools/builders/nogo_change.go index 58cbed052..4b0450dec 100644 --- a/go/tools/builders/nogo_change.go +++ b/go/tools/builders/nogo_change.go @@ -1,85 +1,88 @@ package main + import ( "fmt" "go/token" - "strings" + "os" + "path/filepath" "golang.org/x/tools/go/analysis" ) +// DiagnosticEntry represents a diagnostic entry with the corresponding analyzer. +type DiagnosticEntry struct { + analysis.Diagnostic + *analysis.Analyzer +} + +// An Edit describes the replacement of a portion of a text file. +type Edit struct { + New string `json:"new"` // the replacement + Start int `json:"start"` // starting byte offset of the region to replace + End int `json:"end"` // ending byte offset of the region to replace +} + + // Change represents a set of edits to be applied to a set of files. type Change struct { - AnalysisName string `json:"analysis_name"` - FileToEdits map[string][]Edit `json:"file_to_edits"` + AnalyzerToFileToEdits map[string]map[string][]Edit `json:"analyzer_file_to_edits"` } + + // NewChange creates a new Change object. func NewChange() *Change { return &Change{ - FileToEdits: make(map[string][]Edit), + AnalyzerToFileToEdits: make(map[string]map[string][]Edit), } } -// SetAnalysisName sets the name of the analysis that produced the change. -func (c *Change) SetAnalysisName(name string) { - c.AnalysisName = name -} - -// AddEdit adds an edit to the change. -func (c *Change) AddEdit(file string, edit Edit) { - c.FileToEdits[file] = append(c.FileToEdits[file], edit) -} - -// BuildFromDiagnostics builds a Change from a set of diagnostics. +// NewChangeFromDiagnostics builds a Change from a set of diagnostics. // Unlike Diagnostic, Change is independent of the FileSet given it uses perf-file offsets instead of token.Pos. // This allows Change to be used in contexts where the FileSet is not available, e.g., it remains applicable after it is saved to disk and loaded back. // See https://github.com/golang/tools/blob/master/go/analysis/diagnostic.go for details. -func (c *Change) BuildFromDiagnostics(diagnostics []analysis.Diagnostic, fileSet *token.FileSet) error { - for _, diag := range diagnostics { - for _, sf := range diag.SuggestedFixes { +func NewChangeFromDiagnostics(entries []DiagnosticEntry, fileSet *token.FileSet) (*Change, error) { + c := NewChange() + cwd, err := os.Getwd() // workspace root + if err != nil { + return c, fmt.Errorf("Error getting current working directory: (%v)", err) + } + for _, entry := range entries { + analyzer := entry.Analyzer.Name + for _, sf := range entry.Diagnostic.SuggestedFixes { for _, edit := range sf.TextEdits { file := fileSet.File(edit.Pos) if file == nil { - return fmt.Errorf("invalid fix: missing file info for pos (%v)", edit.Pos) + return c, fmt.Errorf("invalid fix: missing file info for pos (%v)", edit.Pos) } if edit.Pos > edit.End { - return fmt.Errorf("invalid fix: pos (%v) > end (%v)", edit.Pos, edit.End) + return c, fmt.Errorf("invalid fix: pos (%v) > end (%v)", edit.Pos, edit.End) } if eof := token.Pos(file.Base() + file.Size()); edit.End > eof { - return fmt.Errorf("invalid fix: end (%v) past end of file (%v)", edit.End, eof) + return c, fmt.Errorf("invalid fix: end (%v) past end of file (%v)", edit.End, eof) } edit := Edit{Start: file.Offset(edit.Pos), End: file.Offset(edit.End), New: string(edit.NewText)} - fileRelativePath := file.Name() - c.AddEdit(fileRelativePath, edit) + fileRelativePath, err := filepath.Rel(cwd, file.Name()) + if err != nil { + fileRelativePath = file.Name() // fallback logic + } + c.AddEdit(analyzer, fileRelativePath, edit) } } } - return nil + return c, nil } -// MergeChanges merges multiple changes into a single change. -func MergeChanges(changes []Change) Change { - mergedChange := NewChange() // Create a new Change object for the result - analysisNames := []string{} // no deduplication needed - - for _, change := range changes { - if change.AnalysisName != "" { - analysisNames = append(analysisNames, change.AnalysisName) - } - for file, edits := range change.FileToEdits { - // If the file already exists in the merged change, append the edits - if existingEdits, found := mergedChange.FileToEdits[file]; found { - // checking the overlapping of edits happens in edit.go during the ApplyEdits function. - // so we don't need to check it here. - mergedChange.FileToEdits[file] = append(existingEdits, edits...) - } else { - // Otherwise, just set the file and edits - mergedChange.FileToEdits[file] = edits - } - } +// AddEdit adds an edit to the change. +func (c *Change) AddEdit(analyzer string, file string, edit Edit) { + // Check if the analyzer exists in the map + if _, ok := c.AnalyzerToFileToEdits[analyzer]; !ok { + // Initialize the map for the analyzer if it doesn't exist + c.AnalyzerToFileToEdits[analyzer] = make(map[string][]Edit) } - mergedChange.AnalysisName = strings.Join(analysisNames, ",") - return *mergedChange + + // Append the edit to the list of edits for the specific file under the analyzer + c.AnalyzerToFileToEdits[analyzer][file] = append(c.AnalyzerToFileToEdits[analyzer][file], edit) } diff --git a/go/tools/builders/nogo_change_serialization.go b/go/tools/builders/nogo_change_serialization.go index 1b47a341c..4e6acf8af 100644 --- a/go/tools/builders/nogo_change_serialization.go +++ b/go/tools/builders/nogo_change_serialization.go @@ -4,7 +4,7 @@ import ( "encoding/json" "fmt" "io/ioutil" - // "log" + "os" ) // SaveToFile saves the Change struct to a JSON file. @@ -12,11 +12,17 @@ func SaveToFile(filename string, change Change) error { // Serialize Change to JSON jsonData, err := json.MarshalIndent(change, "", " ") if err != nil { - return fmt.Errorf("error serializing to JSON: %v", err) + // needs to write to the bazel-declared output anyway. + errWrite := os.WriteFile(filename, []byte(""), 0644) + if errWrite != nil { + return fmt.Errorf("error serializing to JSON: %v and error writing to the file: %v", err, errWrite) + } else { + return fmt.Errorf("error serializing to JSON: %v", err) + } } - // log.Fatalf("!!!!: %v", change) + // Write the JSON data to the file - err = ioutil.WriteFile(filename, jsonData, 0644) + err = os.WriteFile(filename, jsonData, 0644) if err != nil { return fmt.Errorf("error writing to file: %v", err) } diff --git a/go/tools/builders/nogo_change_serialization_test.go b/go/tools/builders/nogo_change_serialization_test.go new file mode 100644 index 000000000..d8a09f36a --- /dev/null +++ b/go/tools/builders/nogo_change_serialization_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "os" + "reflect" + "testing" +) + +func TestSaveLoad(t *testing.T) { + // Create a temporary file + file, err := os.CreateTemp("", "tmp_file") + if err != nil { + t.Fatal(err) + } + defer os.Remove(file.Name()) + + // Initialize a Change struct with some edits and analyzers + change := *NewChange() + change.AddEdit("AnalyzerA", "file1.txt", Edit{New: "replacement1", Start: 0, End: 5}) + change.AddEdit("AnalyzerA", "file1.txt", Edit{New: "replacement2", Start: 10, End: 15}) + change.AddEdit("AnalyzerB", "file2.txt", Edit{New: "new text", Start: 20, End: 25}) + + // Test saving to file + err = SaveToFile(file.Name(), change) + if err != nil { + t.Fatalf("Failed to save Change struct to file: %v", err) + } + + // Test loading from file + loadedChange, err := LoadFromFile(file.Name()) + if err != nil { + t.Fatalf("Failed to load Change struct from file: %v", err) + } + + // Compare original and loaded Change structs + if !reflect.DeepEqual(change, loadedChange) { + t.Fatalf("Loaded Change struct does not match original.\nOriginal: %+v\nLoaded: %+v", change, loadedChange) + } +} diff --git a/go/tools/builders/nogo_change_test.go b/go/tools/builders/nogo_change_test.go new file mode 100644 index 000000000..3ecfab316 --- /dev/null +++ b/go/tools/builders/nogo_change_test.go @@ -0,0 +1,214 @@ +package main + +import ( + "go/token" + "os" + "path/filepath" + "reflect" + "testing" + + "golang.org/x/tools/go/analysis" +) + +// Mock helper to create a mock file in the token.FileSet +func mockFileSet(fileName string, size int) *token.FileSet { + fset := token.NewFileSet() + f := fset.AddFile(fileName, fset.Base(), size) + for i := 0; i < size; i++ { + f.AddLine(i) + } + return fset +} + +// Mock analyzers for the test +var ( + analyzer1 = &analysis.Analyzer{Name: "analyzer1"} + analyzer2 = &analysis.Analyzer{Name: "analyzer2"} +) + +// TestAddEdit_MultipleAnalyzers tests AddEdit with multiple analyzers and files using reflect.DeepEqual +func TestAddEdit_MultipleAnalyzers(t *testing.T) { + // Step 1: Setup + change := NewChange() + + // Mock data for analyzer 1 + file1 := "file1.go" + edit1a := Edit{Start: 10, End: 20, New: "code1 from analyzer1"} + edit1b := Edit{Start: 30, End: 40, New: "code2 from analyzer1"} + + // Mock data for analyzer 2 + edit2a := Edit{Start: 50, End: 60, New: "code1 from analyzer2"} + edit2b := Edit{Start: 70, End: 80, New: "code2 from analyzer2"} + + // Expected map after all edits are added + expected := map[string]map[string][]Edit{ + analyzer1.Name: { + file1: {edit1a, edit1b}, + }, + analyzer2.Name: { + file1: {edit2a, edit2b}, + }, + } + + // Step 2: Action - Add edits for both analyzers + change.AddEdit(analyzer1.Name, file1, edit1a) + change.AddEdit(analyzer1.Name, file1, edit1b) + change.AddEdit(analyzer2.Name, file1, edit2a) + change.AddEdit(analyzer2.Name, file1, edit2b) + + // Step 3: Verify that the actual map matches the expected map using reflect.DeepEqual + if !reflect.DeepEqual(change.AnalyzerToFileToEdits, expected) { + t.Fatalf("Change.AnalyzerToFileToEdits did not match the expected result.\nGot: %+v\nExpected: %+v", change.AnalyzerToFileToEdits, expected) + } +} + +// Test case for valid, successful cases +func TestNewChangeFromDiagnostics_SuccessCases(t *testing.T) { + cwd, _ := os.Getwd() + file1path := filepath.Join(cwd, "file1.go") + + tests := []struct { + name string + fileSet *token.FileSet + diagnosticEntries []DiagnosticEntry + expectedEdits map[string]map[string][]Edit + }{ + { + name: "ValidEdits", + fileSet: mockFileSet(file1path, 100), + diagnosticEntries: []DiagnosticEntry{ + { + Analyzer: analyzer1, + Diagnostic: analysis.Diagnostic{ + SuggestedFixes: []analysis.SuggestedFix{ + { + TextEdits: []analysis.TextEdit{ + {Pos: token.Pos(5), End: token.Pos(10), NewText: []byte("new_text")}, + }, + }, + }, + }, + }, + { + Analyzer: analyzer1, + Diagnostic: analysis.Diagnostic{ + SuggestedFixes: []analysis.SuggestedFix{ + { + TextEdits: []analysis.TextEdit{ + {Pos: token.Pos(60), End: token.Pos(67), NewText: []byte("new_text")}, + }, + }, + }, + }, + }, + }, + expectedEdits: map[string]map[string][]Edit{ + "analyzer1": { + "file1.go": { + {New: "new_text", Start: 4, End: 9}, // offset is 0-based, while Pos is 1-based + {New: "new_text", Start: 59, End: 66}, // offset is 0-based, while Pos is 1-based + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + change, err := NewChangeFromDiagnostics(tt.diagnosticEntries, tt.fileSet) + + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + + if !reflect.DeepEqual(change.AnalyzerToFileToEdits, tt.expectedEdits) { + t.Fatalf("expected edits: %+v, got: %+v", tt.expectedEdits, change.AnalyzerToFileToEdits) + } + }) + } +} + +// Test case for error cases +func TestNewChangeFromDiagnostics_ErrorCases(t *testing.T) { + cwd, _ := os.Getwd() + file1path := filepath.Join(cwd, "file1.go") + + tests := []struct { + name string + fileSet *token.FileSet + diagnosticEntries []DiagnosticEntry + expectedErr string + }{ + { + name: "InvalidPosEnd", + fileSet: mockFileSet(file1path, 100), + diagnosticEntries: []DiagnosticEntry{ + { + Analyzer: analyzer1, + Diagnostic: analysis.Diagnostic{ + SuggestedFixes: []analysis.SuggestedFix{ + { + TextEdits: []analysis.TextEdit{ + {Pos: token.Pos(15), End: token.Pos(10), NewText: []byte("new_text")}, + }, + }, + }, + }, + }, + }, + expectedErr: "invalid fix: pos (15) > end (10)", + }, + { + name: "EndBeyondFile", + fileSet: mockFileSet(file1path, 100), + diagnosticEntries: []DiagnosticEntry{ + { + Analyzer: analyzer1, + Diagnostic: analysis.Diagnostic{ + SuggestedFixes: []analysis.SuggestedFix{ + { + TextEdits: []analysis.TextEdit{ + {Pos: token.Pos(50), End: token.Pos(102), NewText: []byte("new_text")}, + }, + }, + }, + }, + }, + }, + expectedErr: "invalid fix: end (102) past end of file (101)", // Pos=101 holds the extra EOF token, note Pos is 1-based + }, + { + name: "MissingFileInfo", + fileSet: token.NewFileSet(), // No files added + diagnosticEntries: []DiagnosticEntry{ + { + Analyzer: analyzer1, + Diagnostic: analysis.Diagnostic{ + SuggestedFixes: []analysis.SuggestedFix{ + { + TextEdits: []analysis.TextEdit{ + {Pos: token.Pos(5), End: token.Pos(10), NewText: []byte("new_text")}, + }, + }, + }, + }, + }, + }, + expectedErr: "invalid fix: missing file info for pos (5)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := NewChangeFromDiagnostics(tt.diagnosticEntries, tt.fileSet) + + if err == nil { + t.Fatalf("expected an error, got none") + } + + if err.Error() != tt.expectedErr { + t.Fatalf("expected error: %v, got: %v", tt.expectedErr, err) + } + }) + } +} diff --git a/go/tools/builders/nogo_edit.go b/go/tools/builders/nogo_edit.go deleted file mode 100644 index 6e6d7e580..000000000 --- a/go/tools/builders/nogo_edit.go +++ /dev/null @@ -1,159 +0,0 @@ -/** -Copyright (c) 2009 The Go Authors. All rights reserved. - -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions are -met: - - * Redistributions of source code must retain the above copyright -notice, this list of conditions and the following disclaimer. - * Redistributions in binary form must reproduce the above -copyright notice, this list of conditions and the following disclaimer -in the documentation and/or other materials provided with the -distribution. - * Neither the name of Google Inc. nor the names of its -contributors may be used to endorse or promote products derived from -this software without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -Source: https://sourcegraph.com/github.com/golang/tools/-/blob/internal/diff/diff.go -*/ - -package main - -import ( - "fmt" - "sort" -) - -// An Edit describes the replacement of a portion of a text file. -type Edit struct { - New string `json:"new"` // the replacement - Start int `json:"start"` // starting byte offset of the region to replace - End int `json:"end"` // ending byte offset of the region to replace -} - -func (e Edit) String() string { - return fmt.Sprintf("{Start:%d,End:%d,New:%q}", e.Start, e.End, e.New) -} - -// ApplyEdits applies a sequence of edits to the src buffer and returns the -// result. Edits are applied in order of start offset; edits with the -// same start offset are applied in they order they were provided. -// -// ApplyEdits returns an error if any edit is out of bounds, -// or if any pair of edits is overlapping. -func ApplyEdits(src string, edits []Edit) (string, error) { - edits, size, err := validate(src, edits) - if err != nil { - return "", err - } - - // Apply edits. - out := make([]byte, 0, size) - lastEnd := 0 - for _, edit := range edits { - if lastEnd < edit.Start { - out = append(out, src[lastEnd:edit.Start]...) - } - out = append(out, edit.New...) - lastEnd = edit.End - } - out = append(out, src[lastEnd:]...) - - if len(out) != size { - panic("wrong size") - } - - return string(out), nil -} - -// ApplyEditsBytes is like Apply, but it accepts a byte slice. -// The result is always a new array. -func ApplyEditsBytes(src []byte, edits []Edit) ([]byte, error) { - res, err := ApplyEdits(string(src), edits) - return []byte(res), err -} - -// validate checks that edits are consistent with src, -// and returns the size of the patched output. -// It may return a different slice. -func validate(src string, edits []Edit) ([]Edit, int, error) { - if !sort.IsSorted(editsSort(edits)) { - edits = append([]Edit(nil), edits...) - SortEdits(edits) - } - - // Check validity of edits and compute final size. - size := len(src) - lastEnd := 0 - for _, edit := range edits { - if !(0 <= edit.Start && edit.Start <= edit.End && edit.End <= len(src)) { - return nil, 0, fmt.Errorf("diff has out-of-bounds edits") - } - if edit.Start < lastEnd { - return nil, 0, fmt.Errorf("diff has overlapping edits") - } - size += len(edit.New) + edit.Start - edit.End - lastEnd = edit.End - } - - return edits, size, nil -} - -// UniqueEdits returns a list of edits that is sorted and -// contains no duplicate edits. Returns the index of some -// overlapping adjacent edits if there is one and <0 if the -// edits are valid. -func UniqueEdits(edits []Edit) ([]Edit, int) { - if len(edits) == 0 { - return nil, -1 - } - equivalent := func(x, y Edit) bool { - return x.Start == y.Start && x.End == y.End && x.New == y.New - } - SortEdits(edits) - unique := []Edit{edits[0]} - invalid := -1 - for i := 1; i < len(edits); i++ { - prev, cur := edits[i-1], edits[i] - if !equivalent(prev, cur) { - unique = append(unique, cur) - if prev.End > cur.Start { - invalid = i - } - } - } - return unique, invalid -} - -// SortEdits orders a slice of Edits by (start, end) offset. -// This ordering puts insertions (end = start) before deletions -// (end > start) at the same point, but uses a stable sort to preserve -// the order of multiple insertions at the same point. -// (Apply detects multiple deletions at the same point as an error.) -func SortEdits(edits []Edit) { - sort.Stable(editsSort(edits)) -} - -type editsSort []Edit - -func (a editsSort) Len() int { return len(a) } -func (a editsSort) Less(i, j int) bool { - if cmp := a[i].Start - a[j].Start; cmp != 0 { - return cmp < 0 - } - return a[i].End < a[j].End -} -func (a editsSort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } diff --git a/go/tools/builders/nogo_main.go b/go/tools/builders/nogo_main.go index 4cecceb02..24cf3d338 100644 --- a/go/tools/builders/nogo_main.go +++ b/go/tools/builders/nogo_main.go @@ -77,7 +77,7 @@ func run(args []string) (error, int) { importcfg := flags.String("importcfg", "", "The import configuration file") packagePath := flags.String("p", "", "The package path (importmap) of the package being compiled") xPath := flags.String("x", "", "The archive file where serialized facts should be written") - nogoFixPath := flags.String("fixpath", "", "The fix path for nogo") + nogoFixPath := flags.String("fixpath", "", "The path of the file that stores the nogo fixes") var ignores multiFlag flags.Var(&ignores, "ignore", "Names of files to ignore") flags.Parse(args) @@ -99,7 +99,6 @@ func run(args []string) (error, int) { } } if diagnostics != "" { - // debugMode is defined by the template in generate_nogo_main.go. exitCode := nogoViolation if debugMode { @@ -461,12 +460,8 @@ func (g *goPackage) String() string { // and returns a string containing all the diagnostics that should be printed // to the build log. func checkAnalysisResults(actions []*action, pkg *goPackage, nogoFixPath string) string { - type entry struct { - analysis.Diagnostic - *analysis.Analyzer - } - var diagnostics []entry - var diagnosticsCore []analysis.Diagnostic + var diagnostics []DiagnosticEntry + var errs []error cwd, err := os.Getwd() if cwd == "" || err != nil { @@ -508,7 +503,7 @@ func checkAnalysisResults(actions []*action, pkg *goPackage, nogoFixPath string) if currentConfig.onlyFiles == nil && currentConfig.excludeFiles == nil { for _, diag := range act.diagnostics { - diagnostics = append(diagnostics, entry{Diagnostic: diag, Analyzer: act.a}) + diagnostics = append(diagnostics, DiagnosticEntry{Diagnostic: diag, Analyzer: act.a}) } continue } @@ -546,13 +541,28 @@ func checkAnalysisResults(actions []*action, pkg *goPackage, nogoFixPath string) } } if include { - diagnostics = append(diagnostics, entry{Diagnostic: d, Analyzer: act.a}) + diagnostics = append(diagnostics, DiagnosticEntry{Diagnostic: d, Analyzer: act.a}) } } } if numSkipped > 0 { errs = append(errs, fmt.Errorf("%d analyzers skipped due to type-checking error: %v", numSkipped, pkg.typeCheckError)) } + + if nogoFixPath != "" { + // If the nogo fixes are requested, we need to save the fixes to the file even if they are empty. + // Otherwise, bazel will complain "not all outputs were created or valid" + change, err := NewChangeFromDiagnostics(diagnostics, pkg.fset) + if err != nil { + errs = append(errs, fmt.Errorf("errors in dumping nogo fix, specifically in converting diagnostics to change %v", err)) + } + + err = SaveToFile(nogoFixPath, *change) + if err != nil { + errs = append(errs, fmt.Errorf("errors in dumping nogo fix, specifically in saving the change %v", err)) + } + } + if len(diagnostics) == 0 && len(errs) == 0 { return "" } @@ -560,6 +570,7 @@ func checkAnalysisResults(actions []*action, pkg *goPackage, nogoFixPath string) sort.Slice(diagnostics, func(i, j int) bool { return diagnostics[i].Pos < diagnostics[j].Pos }) + errMsg := &bytes.Buffer{} sep := "" for _, err := range errs { @@ -568,17 +579,10 @@ func checkAnalysisResults(actions []*action, pkg *goPackage, nogoFixPath string) errMsg.WriteString(err.Error()) } for _, d := range diagnostics { - diagnosticsCore = append(diagnosticsCore, d.Diagnostic) - // log.Fatalf("!!!!!: %+v", d.SuggestedFixes) errMsg.WriteString(sep) sep = "\n" fmt.Fprintf(errMsg, "%s: %s (%s)", pkg.fset.Position(d.Pos), d.Message, d.Name) } - - change := NewChange() - change.BuildFromDiagnostics(diagnosticsCore, pkg.fset) - - SaveToFile(nogoFixPath, *change) return errMsg.String() } diff --git a/go/tools/builders/nogo_validation.go b/go/tools/builders/nogo_validation.go index 6738635de..3d164a920 100644 --- a/go/tools/builders/nogo_validation.go +++ b/go/tools/builders/nogo_validation.go @@ -18,7 +18,7 @@ func nogoValidation(args []string) error { if err != nil { return err } - if len(logContent) > 100000000000000000 { + if len(logContent) > 0 { // Separate nogo output from Bazel's --sandbox_debug message via an // empty line. // Don't return to avoid printing the "nogovalidation:" prefix. diff --git a/go/tools/builders/stdlib.go b/go/tools/builders/stdlib.go index 105ca5c63..573144709 100644 --- a/go/tools/builders/stdlib.go +++ b/go/tools/builders/stdlib.go @@ -131,7 +131,7 @@ You may need to use the flags --cpu=x64_windows --compiler=mingw-gcc.`) installArgs = append(installArgs, "-race") } if *pgoprofile != "" { - installArgs = append(installArgs, "-pgo", abs(*pgoprofile)) + gcflags = append(gcflags, "-pgoprofile=" + abs(*pgoprofile)) } if *shared { gcflags = append(gcflags, "-shared")