Skip to content

Commit f5ae225

Browse files
fix: improve interactive lint fixer controls and exit behavior (#196)
1 parent b03d597 commit f5ae225

12 files changed

Lines changed: 591 additions & 51 deletions

File tree

.gitignore

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,6 @@ dist/
1414
# Symlinked from AGENTS.md by mise install
1515
CLAUDE.md
1616

17-
go.work.sum
17+
go.work.sum
18+
19+
.mcp.json

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ openapi spec lint ./spec.yaml
161161
# Lint with custom configuration
162162
openapi spec lint --config lint.yaml ./spec.yaml
163163

164+
# Interactive lint fixing
165+
openapi spec lint --fix-interactive ./spec.yaml
166+
164167
# Bundle external references into components section
165168
openapi spec bundle ./spec.yaml ./bundled-spec.yaml
166169

cmd/openapi/commands/openapi/README.md

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,29 @@ openapi spec lint -c ./lint.yaml ./spec.yaml
6969

7070
# Lint with specific rules disabled
7171
openapi spec lint -d rule-id-1 -d rule-id-2 ./spec.yaml
72+
73+
# Auto-apply non-interactive fixes
74+
openapi spec lint --fix ./spec.yaml
75+
76+
# Interactive fix mode (s=skip, r=skip rule, e/Esc then Enter=exit)
77+
openapi spec lint --fix-interactive ./spec.yaml
78+
79+
# Preview fixes without writing changes
80+
openapi spec lint --fix-interactive --dry-run ./spec.yaml
7281
```
7382

7483
**Flags:**
7584

76-
| Flag | Short | Description |
77-
|------|-------|-------------|
78-
| `--format` | `-f` | Output format: `text` (default) or `json` |
79-
| `--config` | `-c` | Path to lint configuration file |
80-
| `--ruleset` | `-r` | Ruleset to use (default: `all`) |
81-
| `--disable` | `-d` | Rules to disable (can be specified multiple times) |
85+
| Flag | Short | Description |
86+
| ------------------- | ----- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- |
87+
| `--format` | `-f` | Output format: `text` (default) or `json` |
88+
| `--config` | `-c` | Path to lint configuration file |
89+
| `--ruleset` | `-r` | Ruleset to use (default: `all`) |
90+
| `--disable` | `-d` | Rules to disable (can be specified multiple times) |
91+
| `--summary` | | Print a per-rule summary table of findings |
92+
| `--fix` | | Automatically apply non-interactive fixes and write back |
93+
| `--fix-interactive` | | Apply all fixes, prompting for interactive ones (`s`=skip, `r`=skip rule, `e`/Esc then Enter=exit; text prompts support `\` prefix for literal reserved controls) |
94+
| `--dry-run` | | Show what fixes would be applied without changing the file (requires `--fix` or `--fix-interactive`) |
8295

8396
**What lint checks:**
8497

@@ -142,30 +155,30 @@ output_format: text
142155
143156
**Configuration Options:**
144157
145-
| Option | Type | Description |
146-
|--------|------|-------------|
147-
| `extends` | `string[]` | Rulesets to extend from (`all`, `recommended`, `security`) |
148-
| `rules` | `RuleEntry[]` | Individual rule configurations |
149-
| `categories` | `map[string]CategoryConfig` | Category-level configurations |
150-
| `custom_rules` | `CustomRulesConfig` | Custom TypeScript/JavaScript rules |
151-
| `output_format` | `string` | Output format (`text` or `json`) |
158+
| Option | Type | Description |
159+
| --------------- | --------------------------- | ---------------------------------------------------------- |
160+
| `extends` | `string[]` | Rulesets to extend from (`all`, `recommended`, `security`) |
161+
| `rules` | `RuleEntry[]` | Individual rule configurations |
162+
| `categories` | `map[string]CategoryConfig` | Category-level configurations |
163+
| `custom_rules` | `CustomRulesConfig` | Custom TypeScript/JavaScript rules |
164+
| `output_format` | `string` | Output format (`text` or `json`) |
152165

153166
**Available Rulesets:**
154167

155-
| Ruleset | Description |
156-
|---------|-------------|
157-
| `all` | All available rules (default) |
168+
| Ruleset | Description |
169+
| ------------- | ------------------------------------------------------------------ |
170+
| `all` | All available rules (default) |
158171
| `recommended` | Balanced ruleset - semantic rules, essential style, basic security |
159-
| `security` | Comprehensive OWASP security rules |
172+
| `security` | Comprehensive OWASP security rules |
160173

161174
**Rule Entry Options:**
162175

163-
| Option | Type | Description |
164-
|--------|------|-------------|
165-
| `id` | `string` | Exact rule ID to configure |
166-
| `match` | `string` | Regex pattern to match rule IDs |
167-
| `severity` | `string` | `error`, `warning`, or `hint` |
168-
| `disabled` | `bool` | Set to `true` to disable the rule |
176+
| Option | Type | Description |
177+
| ---------- | -------- | --------------------------------- |
178+
| `id` | `string` | Exact rule ID to configure |
179+
| `match` | `string` | Regex pattern to match rule IDs |
180+
| `severity` | `string` | `error`, `warning`, or `hint` |
181+
| `disabled` | `bool` | Set to `true` to disable the rule |
169182

170183
#### Custom Rules
171184

@@ -266,16 +279,16 @@ openapi spec lint -c ./lint.yaml ./spec.yaml
266279

267280
Your rule class must implement the `RuleRunner` interface:
268281

269-
| Method | Required | Description |
270-
|--------|----------|-------------|
271-
| `id()` | Yes | Unique rule identifier |
272-
| `category()` | Yes | Rule category (e.g., `style`, `security`) |
273-
| `description()` | Yes | Full description of the rule |
274-
| `summary()` | Yes | Short summary for output |
275-
| `link()` | No | URL to documentation |
276-
| `defaultSeverity()` | No | Default severity (`error`, `warning`, `hint`) |
277-
| `versions()` | No | OpenAPI versions this rule applies to |
278-
| `run()` | Yes | Execute the rule and return validation errors |
282+
| Method | Required | Description |
283+
| ------------------- | -------- | --------------------------------------------- |
284+
| `id()` | Yes | Unique rule identifier |
285+
| `category()` | Yes | Rule category (e.g., `style`, `security`) |
286+
| `description()` | Yes | Full description of the rule |
287+
| `summary()` | Yes | Short summary for output |
288+
| `link()` | No | URL to documentation |
289+
| `defaultSeverity()` | No | Default severity (`error`, `warning`, `hint`) |
290+
| `versions()` | No | OpenAPI versions this rule applies to |
291+
| `run()` | Yes | Execute the rule and return validation errors |
279292

280293
**Accessing Document Data:**
281294

@@ -1245,10 +1258,10 @@ openapi spec query --format json 'schemas | where(isComponent) | take(5)' ./spec
12451258

12461259
**Flags:**
12471260

1248-
| Flag | Short | Description |
1249-
|------|-------|-------------|
1250-
| `--format` | | Output format: `table` (default), `json`, `markdown`, or `toon` |
1251-
| `--file` | `-f` | Read query from file instead of argument |
1261+
| Flag | Short | Description |
1262+
| ---------- | ----- | --------------------------------------------------------------- |
1263+
| `--format` | | Output format: `table` (default), `json`, `markdown`, or `toon` |
1264+
| `--file` | `-f` | Read query from file instead of argument |
12521265

12531266
For the full query language reference, run `openapi spec query-reference`.
12541267

cmd/openapi/commands/openapi/lint.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func init() {
9999
lintCmd.Flags().StringSliceVarP(&lintDisableRules, "disable", "d", nil, "Rule IDs to disable (can be repeated)")
100100
lintCmd.Flags().BoolVar(&lintSummary, "summary", false, "Print a per-rule summary table of findings")
101101
lintCmd.Flags().BoolVar(&lintFix, "fix", false, "Automatically apply non-interactive fixes and write back")
102-
lintCmd.Flags().BoolVar(&lintFixInteractive, "fix-interactive", false, "Apply all fixes, prompting for interactive ones")
102+
lintCmd.Flags().BoolVar(&lintFixInteractive, "fix-interactive", false, "Apply all fixes, prompting for interactive ones (s=skip, r=skip rule, e/Esc then Enter=exit; text prompts: prefix \\ for literal)")
103103
lintCmd.Flags().BoolVar(&lintDryRun, "dry-run", false, "Show what fixes would be applied without changing the file (requires --fix or --fix-interactive)")
104104
}
105105

jsonschema/oas3/tests/testsuite

Submodule testsuite updated 76 files

linter/fix/engine.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,22 @@ func (e *Engine) ProcessErrors(ctx context.Context, doc *openapi.OpenAPI, errs [
157157

158158
result := &Result{}
159159
modified := make(map[conflictKey]bool)
160+
skippedRules := make(map[string]bool)
160161

161162
for _, fe := range fixable {
162163
fix := fe.fix
163164
vErr := fe.vErr
164165

166+
// Skip any remaining fixes for rules the user opted to skip.
167+
if skippedRules[vErr.Rule] {
168+
result.Skipped = append(result.Skipped, SkippedFix{
169+
Error: vErr,
170+
Fix: fix,
171+
Reason: SkipUser,
172+
})
173+
continue
174+
}
175+
165176
// Check for conflicts at the same location
166177
key := conflictKey{Line: vErr.GetLineNumber(), Column: vErr.GetColumnNumber(), Rule: vErr.Rule}
167178
if key.Line >= 0 && modified[key] {
@@ -204,6 +215,18 @@ func (e *Engine) ProcessErrors(ctx context.Context, doc *openapi.OpenAPI, errs [
204215
})
205216
continue
206217
}
218+
if errors.Is(err, validation.ErrSkipRule) {
219+
skippedRules[vErr.Rule] = true
220+
result.Skipped = append(result.Skipped, SkippedFix{
221+
Error: vErr,
222+
Fix: fix,
223+
Reason: SkipUser,
224+
})
225+
continue
226+
}
227+
if errors.Is(err, validation.ErrExitInteractive) {
228+
return result, nil
229+
}
207230
result.Failed = append(result.Failed, FailedFix{
208231
Error: vErr, Fix: fix, FixError: err,
209232
})

linter/fix/engine_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,12 @@ type mockPrompter struct {
7474
responses []string
7575
err error
7676
called bool
77+
callCount int
7778
}
7879

7980
func (p *mockPrompter) PromptFix(_ *validation.Error, _ validation.Fix) ([]string, error) {
8081
p.called = true
82+
p.callCount++
8183
return p.responses, p.err
8284
}
8385

@@ -180,6 +182,64 @@ func TestEngine_ModeInteractive_UserSkips(t *testing.T) {
180182
assert.False(t, f.applied, "fix should not have been applied")
181183
}
182184

185+
func TestEngine_ModeInteractive_SkipRuleSkipsRemainingSameRule(t *testing.T) {
186+
t.Parallel()
187+
188+
f1 := &mockInteractiveFix{
189+
description: "needs input 1",
190+
prompts: []validation.Prompt{{Type: validation.PromptFreeText, Message: "enter value"}},
191+
}
192+
f2 := &mockInteractiveFix{
193+
description: "needs input 2",
194+
prompts: []validation.Prompt{{Type: validation.PromptFreeText, Message: "enter value"}},
195+
}
196+
fOtherRule := &mockFix{description: "other rule fix"}
197+
198+
prompter := &mockPrompter{err: validation.ErrSkipRule}
199+
engine := fix.NewEngine(fix.Options{Mode: fix.ModeInteractive}, prompter, nil)
200+
result, err := engine.ProcessErrors(t.Context(), &openapi.OpenAPI{}, []error{
201+
makeError("test-rule", 1, 1, "issue 1", f1),
202+
makeError("test-rule", 2, 1, "issue 2", f2),
203+
makeError("other-rule", 3, 1, "issue 3", fOtherRule),
204+
})
205+
206+
require.NoError(t, err, "ProcessErrors should not fail")
207+
assert.Len(t, result.Applied, 1, "should still apply fixes from other rules")
208+
assert.True(t, fOtherRule.applied, "other rule fix should have been applied")
209+
assert.Len(t, result.Skipped, 2, "should skip current and remaining fixes for the rule")
210+
assert.False(t, f1.applied, "first skipped fix should not be applied")
211+
assert.False(t, f2.applied, "remaining same-rule fix should not be applied")
212+
assert.True(t, prompter.called, "prompter should have been called")
213+
assert.Equal(t, 1, prompter.callCount, "prompter should only be called for the first same-rule fix")
214+
}
215+
216+
func TestEngine_ModeInteractive_ExitReturnsPartialResults(t *testing.T) {
217+
t.Parallel()
218+
219+
fApplied := &mockFix{description: "auto fix before exit"}
220+
fExit := &mockInteractiveFix{
221+
description: "needs input",
222+
prompts: []validation.Prompt{{Type: validation.PromptFreeText, Message: "enter value"}},
223+
}
224+
fNotProcessed := &mockFix{description: "after exit"}
225+
226+
prompter := &mockPrompter{err: validation.ErrExitInteractive}
227+
engine := fix.NewEngine(fix.Options{Mode: fix.ModeInteractive}, prompter, nil)
228+
result, err := engine.ProcessErrors(t.Context(), &openapi.OpenAPI{}, []error{
229+
makeError("rule-a", 1, 1, "issue 1", fApplied),
230+
makeError("rule-b", 2, 1, "issue 2", fExit),
231+
makeError("rule-c", 3, 1, "issue 3", fNotProcessed),
232+
})
233+
234+
require.NoError(t, err, "ProcessErrors should not fail")
235+
require.Len(t, result.Applied, 1, "should keep already-applied fixes")
236+
assert.True(t, fApplied.applied, "first fix should be applied")
237+
assert.False(t, fExit.applied, "interactive fix should not be applied after exit")
238+
assert.False(t, fNotProcessed.applied, "fixes after exit should not be processed")
239+
assert.Empty(t, result.Failed, "exit should not be treated as failure")
240+
assert.Equal(t, 1, prompter.callCount, "prompter should only be called once before exiting")
241+
}
242+
183243
func TestEngine_DryRun(t *testing.T) {
184244
t.Parallel()
185245

linter/fix/terminal_prompter.go

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io"
77
"strconv"
88
"strings"
9+
"unicode/utf8"
910

1011
"github.com/speakeasy-api/openapi/validation"
1112
)
@@ -66,6 +67,8 @@ func (p *TerminalPrompter) promptChoice(prompt validation.Prompt) (string, error
6667
p.writef(" [%d] %s\n", j+1, choice)
6768
}
6869
p.writef(" [s] Skip\n")
70+
p.writef(" [r] Skip remaining fixes for this rule\n")
71+
p.writef(" [e] Exit interactive fixing\n")
6972

7073
for {
7174
if prompt.Default != "" {
@@ -80,8 +83,17 @@ func (p *TerminalPrompter) promptChoice(prompt validation.Prompt) (string, error
8083
}
8184
line = strings.TrimSpace(line)
8285

83-
if line == "s" || line == "skip" {
86+
switch strings.ToLower(line) {
87+
case "s", "skip":
8488
return "", validation.ErrSkipFix
89+
case "r":
90+
return "", validation.ErrSkipRule
91+
case "e", "exit":
92+
return "", validation.ErrExitInteractive
93+
}
94+
95+
if isEscapeInput(line) {
96+
return "", validation.ErrExitInteractive
8597
}
8698

8799
if line == "" && prompt.Default != "" {
@@ -90,7 +102,7 @@ func (p *TerminalPrompter) promptChoice(prompt validation.Prompt) (string, error
90102

91103
idx, err := strconv.Atoi(line)
92104
if err != nil || idx < 1 || idx > len(prompt.Choices) {
93-
p.writef(" Invalid choice: %s (enter 1-%d or s to skip)\n", line, len(prompt.Choices))
105+
p.writef(" Invalid choice: %s (enter 1-%d, s, r, e, or Esc then Enter)\n", line, len(prompt.Choices))
94106
continue
95107
}
96108

@@ -103,16 +115,29 @@ func (p *TerminalPrompter) promptFreeText(prompt validation.Prompt) (string, err
103115
if prompt.Default != "" {
104116
p.writef(" (default: %s)", prompt.Default)
105117
}
106-
p.writef(" [s to skip]: ")
118+
p.writef(" [s=skip, r=skip rule, e=exit; prefix \\ for literal]: ")
107119

108120
line, err := p.reader.ReadString('\n')
109121
if err != nil {
110122
return "", fmt.Errorf("reading input: %w", err)
111123
}
112124
line = strings.TrimSpace(line)
113125

114-
if line == "s" || line == "skip" {
115-
return "", validation.ErrSkipFix
126+
if unescaped, ok := unescapeReservedControlInput(line); ok {
127+
line = unescaped
128+
} else {
129+
switch strings.ToLower(line) {
130+
case "s", "skip":
131+
return "", validation.ErrSkipFix
132+
case "r":
133+
return "", validation.ErrSkipRule
134+
case "e", "exit":
135+
return "", validation.ErrExitInteractive
136+
}
137+
138+
if isEscapeInput(line) {
139+
return "", validation.ErrExitInteractive
140+
}
116141
}
117142

118143
if line == "" && prompt.Default != "" {
@@ -126,6 +151,33 @@ func (p *TerminalPrompter) promptFreeText(prompt validation.Prompt) (string, err
126151
return line, nil
127152
}
128153

154+
func unescapeReservedControlInput(line string) (string, bool) {
155+
if !strings.HasPrefix(line, "\\") {
156+
return "", false
157+
}
158+
159+
remainder := strings.TrimPrefix(line, "\\")
160+
switch strings.ToLower(remainder) {
161+
case "s", "skip", "r", "e", "exit":
162+
return remainder, true
163+
}
164+
165+
if isEscapeInput(remainder) {
166+
return remainder, true
167+
}
168+
169+
return "", false
170+
}
171+
172+
func isEscapeInput(line string) bool {
173+
if line == "" {
174+
return false
175+
}
176+
177+
r, size := utf8.DecodeRuneInString(line)
178+
return r == '\x1b' && size == len(line)
179+
}
180+
129181
func (p *TerminalPrompter) Confirm(message string) (bool, error) {
130182
p.writef("%s [y/n]: ", message)
131183

0 commit comments

Comments
 (0)