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..10a2354b3 100644 --- a/go/private/rules/library.bzl +++ b/go/private/rules/library.bzl @@ -49,6 +49,7 @@ def _go_library_impl(ctx): archive = go.archive(go, source) validation_output = archive.data._validation_output nogo_fix_output = archive.data._out_nogo_fix + nogo_fix_output = archive.data._out_nogo_fix return [ library, @@ -66,7 +67,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..6b4970315 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,9 @@ 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: + 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 @@ -95,6 +99,8 @@ def _go_test_impl(ctx): external_archive = go.archive(go, external_source, is_external_pkg = True) if external_archive.data._validation_output: validation_outputs.append(external_archive.data._validation_output) + if external_archive.data._out_nogo_fix: + nogo_fix_outputs.append(external_archive.data._out_nogo_fix) # now generate the main function repo_relative_rundir = ctx.attr.rundir or ctx.label.package or "." @@ -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/tools/builders/BUILD.bazel b/go/tools/builders/BUILD.bazel index 707e41ac9..c003f2fc3 100644 --- a/go/tools/builders/BUILD.bazel +++ b/go/tools/builders/BUILD.bazel @@ -31,6 +31,17 @@ go_test( ], ) +go_test( + name = "nogo_change_test", + size = "small", + srcs = [ + "nogo_change.go", + "nogo_change_test.go", + "nogo_change_serialization.go", + "nogo_change_serialization_test.go", + ], +) + go_test( name = "stdliblist_test", size = "small", @@ -55,6 +66,9 @@ go_test( "nolint.go", "nolint_test.go", ], + deps = [ + "@org_golang_x_tools//go/analysis", + ], ) filegroup( @@ -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/go-code.code-workspace b/go/tools/builders/go-code.code-workspace new file mode 100644 index 000000000..2e7f4b59c --- /dev/null +++ b/go/tools/builders/go-code.code-workspace @@ -0,0 +1,11 @@ +{ + "folders": [ + { + "path": "../../../../go-code" + }, + { + "path": "../../.." + } + ], + "settings": {} +} diff --git a/go/tools/builders/nogo.go b/go/tools/builders/nogo.go index cdf48b5ff..ae5925009 100644 --- a/go/tools/builders/nogo.go +++ b/go/tools/builders/nogo.go @@ -41,7 +41,7 @@ func nogo(args []string) error { 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 @@ -106,9 +106,6 @@ func runNogo(workDir string, nogoPath string, srcs, ignores []string, facts []ar 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..4aded5955 100644 --- a/go/tools/builders/nogo_change.go +++ b/go/tools/builders/nogo_change.go @@ -1,85 +1,90 @@ 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() + // TODO: check whether cwd is proper. + cwd, err := os.Getwd() // sandbox 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..af6dc60cb 100644 --- a/go/tools/builders/nogo_change_serialization.go +++ b/go/tools/builders/nogo_change_serialization.go @@ -1,10 +1,11 @@ package main + import ( "encoding/json" "fmt" "io/ioutil" - // "log" + "os" ) // SaveToFile saves the Change struct to a JSON file. @@ -14,9 +15,9 @@ func SaveToFile(filename string, change Change) error { if err != nil { 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..72af14fce 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 { @@ -457,16 +456,13 @@ func (g *goPackage) String() string { return g.types.Path() } + + // checkAnalysisResults checks the analysis diagnostics in the given actions // 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 +504,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,7 +542,7 @@ 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}) } } } @@ -560,6 +556,19 @@ func checkAnalysisResults(actions []*action, pkg *goPackage, nogoFixPath string) sort.Slice(diagnostics, func(i, j int) bool { return diagnostics[i].Pos < diagnostics[j].Pos }) + + if nogoFixPath != "" { + 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)) + } + } + errMsg := &bytes.Buffer{} sep := "" for _, err := range errs { @@ -568,17 +577,12 @@ 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() }