diff --git a/CHANGELOG.md b/CHANGELOG.md index be7fc30..3e7adbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 reach files that were never opened. An open buffer always shadows its disk copy, and watched-file events together with a per-query freshness check keep the index current. +- **Quick fixes for the textflag include and the argument area.** The + `missing-textflag-include` warning offers to add the include after the + last one in the file, and the `abi-argsize` warning offers to set the + TEXT argument area to the size the `// func` signature implies, computed + by the new `lint.ExpectedArgSize`. ### Fixed diff --git a/README.md b/README.md index f4284f3..3855802 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,8 @@ to give that syntax the tooling it deserves. help, document highlights, workspace symbol search, #include document links and folding ranges over stdio; definition, references and rename work across every open document and the indexed workspace files beyond - them. + them, and the quick fixes add a missing textflag.h include and set the + argument area from the // func signature. - **Comparators and audits.** `gasm diff` compares the machine code of two assembly files byte-for-byte, `gasm profile` shows basic-block structure, `gasm audit-instructions` diffs the encoder against the installed toolchain, diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 75f049f..237621b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -254,7 +254,10 @@ document store, republishes diagnostics on every change, and provides: - **assists**: document formatting through the `format` package, inlay hints (the frame size after the TEXT argument area), signature help (the callee's `// func` signature while the cursor is on a `CALL`), and code actions - offering quick fixes for the `missing-ret` and `unused-label` diagnostics. + offering quick fixes for the `missing-ret` and `unused-label` diagnostics, + the `missing-textflag-include` warning (the include after the last one in + the file) and the `abi-argsize` warning (the argument area set to the size + the `// func` signature implies). - **document information**: pull diagnostics (`textDocument/diagnostic`), #include document links (resolved against the document directory, then `$GOROOT/pkg/include`) and folding ranges (one collapsible region per diff --git a/docs/CLI.md b/docs/CLI.md index 3a6157c..0aa5b44 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -458,7 +458,10 @@ function bodies. Definition, references and rename work across every open document and the wider workspace on disk: the server indexes the `.s` files under the workspace root that the editor has never opened, an open buffer always shadows its disk copy, and watched-file events together with a -per-query freshness check keep the index current. +per-query freshness check keep the index current. The quick fixes add the +missing `#include "textflag.h"`, set the TEXT argument area to the size the +`// func` signature implies, add a missing `RET`, and remove an unused +label. ## version diff --git a/lint/abi.go b/lint/abi.go index b2e7de8..bf517aa 100644 --- a/lint/abi.go +++ b/lint/abi.go @@ -11,6 +11,15 @@ import ( "strings" ) +// ExpectedArgSize computes the argument-area size (parameters plus results, +// laid out with Go's alignment rules on a 64-bit target) implied by the +// `// func …` signature in a doc comment. It returns ok=false when there is +// no parseable signature or it uses a type whose size cannot be determined +// (a named type), so the caller can skip the fix rather than guess. +func ExpectedArgSize(doc string) (int64, bool) { + return abiExpectedArgSize(doc) +} + // abiExpectedArgSize computes the argument-area size (parameters plus results, // laid out with Go's alignment rules on a 64-bit target) from the `// func …` // signature in a TEXT function's doc comment. It returns ok=false when there diff --git a/lsp/handlers.go b/lsp/handlers.go index fefdc98..ee2cda7 100644 --- a/lsp/handlers.go +++ b/lsp/handlers.go @@ -11,6 +11,7 @@ import ( "regexp" "runtime" "slices" + "strconv" "strings" "unicode" "unicode/utf16" @@ -386,6 +387,61 @@ func (s *Server) codeActions(p codeActionParams) []CodeAction { } } } + case "missing-textflag-include": + // Offer to #include the header that defines the flag macros: + // after the last existing include, or at the top of the file + // when there is none. + insertLine := 0 + lines := strings.Split(text, "\n") + for i, line := range lines { + if includeRe.MatchString(line) { + insertLine = i + 1 + } + } + actions = append(actions, CodeAction{ + Title: `Add #include "textflag.h"`, + Kind: "quickfix", + Edit: &WorkspaceEdit{ + Changes: map[string][]TextEdit{p.TextDocument.URI: {{ + Range: Range{Start: Position{Line: insertLine, Character: 0}, End: Position{Line: insertLine, Character: 0}}, + NewText: "#include \"textflag.h\"\n", + }}}, + }, + }) + case "abi-argsize": + // Offer to set the TEXT argument area to the size the // func + // signature implies. The diagnostic's range covers the TEXT + // keyword, so a line match picks the function it belongs to. + f, _ := parser.Parse(uriPath(p.TextDocument.URI), text) + if f == nil { + continue + } + for _, d := range f.Decls { + t, ok := d.(*ast.Text) + if !ok || t.Keyword.Pos.Line-1 != int(diag.Range.Start.Line) { + continue + } + want, ok := lint.ExpectedArgSize(t.Doc) + if !ok || t.Args == nil || !t.Args.Imm.HasVal || t.Args.Imm.Val == want { + continue + } + // The parser records the argument area with its leading + // minus, so the replacement spans the whole `-16` shape. + rng := clientRange(text, Range{ + Start: Position{Line: t.Args.Pos.Line - 1, Character: t.Args.Pos.Column - 1}, + End: Position{Line: t.Args.Pos.Line - 1, Character: t.Args.Pos.Column - 1 + runeLen(t.Args.Raw)}, + }) + actions = append(actions, CodeAction{ + Title: fmt.Sprintf("Set arg size to %d", want), + Kind: "quickfix", + Edit: &WorkspaceEdit{ + Changes: map[string][]TextEdit{p.TextDocument.URI: {{ + Range: rng, + NewText: "-" + strconv.FormatInt(want, 10), + }}}, + }, + }) + } } } return actions diff --git a/lsp/server_test.go b/lsp/server_test.go index d9be465..7b57f06 100644 --- a/lsp/server_test.go +++ b/lsp/server_test.go @@ -1014,6 +1014,134 @@ func TestCodeActionsTargetsFlaggedFunctionOnly(t *testing.T) { } } +// TestCodeActionInsertsTextflagInclude offers the include a file must carry +// when it uses flag macros: at the top of the file when there is no include +// yet, after the last one otherwise. +func TestCodeActionInsertsTextflagInclude(t *testing.T) { + doc := "TEXT \u00b7f(SB), NOSPLIT, $0\n\tRET\n" + withInclude := "#include \"go_asm.h\"\n\nTEXT \u00b7f(SB), NOSPLIT, $0\n\tRET\n" + in := session("file:///f_amd64.s", doc) + + frame(30, "textDocument/codeAction", map[string]any{ + "textDocument": map[string]any{"uri": "file:///f_amd64.s"}, + "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 1, "character": 0}}, + "context": map[string]any{"diagnostics": []map[string]any{ + {"code": "missing-textflag-include", "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 0, "character": 4}}}, + }}, + }) + + frame(nil, "textDocument/didChange", map[string]any{ + "textDocument": map[string]any{"uri": "file:///f_amd64.s", "version": 2}, + "contentChanges": []map[string]any{{"text": withInclude}}, + }) + + frame(31, "textDocument/codeAction", map[string]any{ + "textDocument": map[string]any{"uri": "file:///f_amd64.s"}, + "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 3, "character": 0}}, + "context": map[string]any{"diagnostics": []map[string]any{ + {"code": "missing-textflag-include", "range": map[string]any{"start": map[string]any{"line": 2, "character": 0}, "end": map[string]any{"line": 2, "character": 4}}}, + }}, + }) + + frame(nil, "exit", nil) + msgs := run(t, in) + + resp := findByID(msgs, 30) + if resp == nil { + t.Fatal("no codeAction response") + } + var actions []CodeAction + if err := json.Unmarshal(mustResult(t, resp), &actions); err != nil { + t.Fatal(err) + } + if len(actions) != 1 { + t.Fatalf("actions = %d, want 1", len(actions)) + } + edits := actions[0].Edit.Changes["file:///f_amd64.s"] + if len(edits) != 1 || edits[0].Range.Start.Line != 0 || edits[0].Range.Start.Character != 0 { + t.Fatalf("edit = %+v, want an insertion at line 0 character 0", edits) + } + if got := applyEdits(doc, edits); !strings.HasPrefix(got, "#include \"textflag.h\"\nTEXT \u00b7f") { + t.Errorf("edited document = %q, want the include as the first line", got) + } + + resp = findByID(msgs, 31) + if resp == nil { + t.Fatal("no codeAction response for the with-include variant") + } + actions = nil + if err := json.Unmarshal(mustResult(t, resp), &actions); err != nil { + t.Fatal(err) + } + if len(actions) != 1 || actions[0].Edit.Changes["file:///f_amd64.s"][0].Range.Start.Line != 1 { + t.Fatalf("actions = %+v, want one insertion after the last include (line 1)", actions) + } +} + +// TestCodeActionFixesArgSize offers the argument area the // func signature +// implies; without a parseable signature there is nothing to offer. +func TestCodeActionFixesArgSize(t *testing.T) { + doc := "// func Add(x int64) int64\n" + + "TEXT \u00b7Add(SB), NOSPLIT, $0-8\n" + + "\tMOVQ x+0(FP), AX\n" + + "\tADDQ y+8(FP), AX\n" + + "\tMOVQ AX, ret+16(FP)\n" + + "\tRET\n" + bare := "TEXT \u00b7Bare(SB), NOSPLIT, $0-8\n\tRET\n" + in := session("file:///f_amd64.s", doc) + + frame(32, "textDocument/codeAction", map[string]any{ + "textDocument": map[string]any{"uri": "file:///f_amd64.s"}, + "range": map[string]any{"start": map[string]any{"line": 1, "character": 0}, "end": map[string]any{"line": 1, "character": 4}}, + "context": map[string]any{"diagnostics": []map[string]any{ + {"code": "abi-argsize", "range": map[string]any{"start": map[string]any{"line": 1, "character": 0}, "end": map[string]any{"line": 1, "character": 4}}}, + }}, + }) + + frame(nil, "textDocument/didChange", map[string]any{ + "textDocument": map[string]any{"uri": "file:///f_amd64.s", "version": 2}, + "contentChanges": []map[string]any{{"text": bare}}, + }) + + frame(33, "textDocument/codeAction", map[string]any{ + "textDocument": map[string]any{"uri": "file:///f_amd64.s"}, + "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 0, "character": 4}}, + "context": map[string]any{"diagnostics": []map[string]any{ + {"code": "abi-argsize", "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 0, "character": 4}}}, + }}, + }) + + frame(nil, "exit", nil) + msgs := run(t, in) + + resp := findByID(msgs, 32) + if resp == nil { + t.Fatal("no codeAction response") + } + var actions []CodeAction + if err := json.Unmarshal(mustResult(t, resp), &actions); err != nil { + t.Fatal(err) + } + if len(actions) != 1 { + t.Fatalf("actions = %d, want 1", len(actions)) + } + if actions[0].Title != "Set arg size to 16" { + t.Errorf("action title = %q, want Set arg size to 16", actions[0].Title) + } + edits := actions[0].Edit.Changes["file:///f_amd64.s"] + if len(edits) != 1 || edits[0].NewText != "-16" || edits[0].Range.Start.Line != 1 { + t.Fatalf("edit = %+v, want -16 on line 1", edits) + } + if got := applyEdits(doc, edits); !strings.Contains(got, "$0-16") { + t.Errorf("edited document = %q, want $0-16", got) + } + + // No // func signature, no offer: the expected size cannot be computed. + resp = findByID(msgs, 33) + if resp == nil { + t.Fatal("no codeAction response for the bare variant") + } + actions = nil + if err := json.Unmarshal(mustResult(t, resp), &actions); err != nil { + t.Fatal(err) + } + if len(actions) != 0 { + t.Fatalf("actions = %+v, want none without a // func signature", actions) + } +} + // TestCrossFileDefinitionAndReferences opens two documents: docA calls // ·helper(SB), docB defines it. Definition must jump to docB and references // must collect the call site in docA plus the definition in docB.