Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .claude/agents/developer.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,7 @@ servers := []*parser.Server{
}

// Deep copy when modifying to avoid mutations
data, _ := json.Marshal(original)
var copy MyType
json.Unmarshal(data, &copy)
copy := original.DeepCopy()
```

## Testing Requirements
Expand Down
2 changes: 1 addition & 1 deletion .claude/docs/oas-concepts.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if typeStr, ok := schema.Type.(string); ok {
1. **Assuming schema.Type is always a string** - Use type assertions and handle both string and []string cases
2. **Creating value slices instead of pointer slices** - Check parser types and use `&Type{...}` syntax
3. **Forgetting to track conversion issues** - Add issues for every lossy conversion or unsupported feature
4. **Mutating source documents** - Always deep copy before modification (use JSON marshal/unmarshal)
4. **Mutating source documents** - Always deep copy before modification using generated `DeepCopy()` methods (e.g., `doc.DeepCopy()`). Never use JSON marshal/unmarshal — it loses `interface{}` type distinctions and drops `json:"-"` fields
5. **Not handling operation-level consumes/produces** - Check operation-level first, then fall back to document-level
6. **Ignoring version-specific features during conversion** - Explicitly check and warn about features that don't convert
7. **Confusing Version (string) with OASVersion (enum)** - `ParseResult` has TWO version fields:
Expand Down
2 changes: 1 addition & 1 deletion .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ if typeStr, ok := schema.Type.(string); ok {
1. **Assuming schema.Type is always a string** - Use type assertions and handle both string and []string cases
2. **Creating value slices instead of pointer slices** - Check parser types and use `&Type{...}` syntax
3. **Forgetting to track conversion issues** - Add issues for every lossy conversion or unsupported feature
4. **Mutating source documents** - Always deep copy before modification (use JSON marshal/unmarshal)
4. **Mutating source documents** - Deep copy before modification using generated `DeepCopy()` methods on parser types (e.g., `doc.DeepCopy()`). Never use JSON marshal/unmarshal for deep copying — it silently loses `interface{}` type distinctions (e.g., `string` vs `[]string` in OAS 3.1 `schema.Type`) and drops fields tagged `json:"-"`. When the caller owns the document and won't reuse it, use `WithMutableInput(true)` to skip the copy entirely.
5. **Not handling operation-level consumes/produces** - Check operation-level first, then fall back to document-level
6. **Ignoring version-specific features during conversion** - Explicitly check and warn about features that don't convert

Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ For documentation-only changes, only items 3, 6, and 7 apply.
1. **Type assertions:** Don't assume `schema.Type` is always a string - check both string and []string
2. **Pointer slices:** Use `&Type{...}` for pointer semantics, not value types
3. **Conversion issues:** Track all lossy conversions or unsupported features
4. **Deep copy:** Never mutate source documents - use JSON marshal/unmarshal
4. **Deep copy:** Never mutate source documents - use generated `DeepCopy()` methods (e.g., `doc.DeepCopy()`), never JSON marshal/unmarshal
5. **Operation-level overrides:** Check operation-level fields before falling back to document-level
6. **Version features:** Explicitly handle version-specific features during conversion

Expand Down
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
- **Use constants**: `httputil.MethodGet`, `severity.SeverityError`
- **Always run `go_diagnostics`** after edits—hints improve perf 5-15%
- **Favor fixing immediately** over deferring issues
- **Deep copy**: Use generated `doc.DeepCopy()` methods, **never** JSON marshal/unmarshal (loses `interface{}` types, drops `json:"-"` fields)
- **`make check` before pushing** — not just `go test`; catches lint, formatting, and trailing whitespace
- **`docs/` is generated**: Files are copied by mkdocs build scripts — always edit source files at repo root

## Orchestrator Mode

Expand Down
4 changes: 1 addition & 3 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -614,9 +614,7 @@ modified := sourceDoc
modified.Info.Title = "New Title" // Changes sourceDoc too!

// ✅ Correct - deep copy first
data, _ := json.Marshal(sourceDoc)
var modified parser.OAS3Document
json.Unmarshal(data, &modified)
modified := sourceDoc.DeepCopy()
modified.Info.Title = "New Title" // Safe
```

Expand Down
2 changes: 2 additions & 0 deletions cmd/oastools/commands/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ func HandleFix(args []string) error {
f.GenericNamingConfig = genericConfig
f.OperationIdNamingConfig = operationIdConfig
f.DryRun = flags.DryRun
f.MutableInput = true
if flags.StubResponseDesc != "" {
f.StubConfig.ResponseDescription = flags.StubResponseDesc
}
Expand Down Expand Up @@ -264,6 +265,7 @@ func HandleFix(args []string) error {
fixer.WithGenericNamingConfig(genericConfig),
fixer.WithOperationIdNamingConfig(operationIdConfig),
fixer.WithDryRun(flags.DryRun),
fixer.WithMutableInput(true),
}
if parseResult.SourceMap != nil {
fixOpts = append(fixOpts, fixer.WithSourceMap(parseResult.SourceMap))
Expand Down
12 changes: 6 additions & 6 deletions fixer/fixer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package fixer

import (
"fmt"
"slices"

"github.com/erraggy/oastools/parser"
)
Expand Down Expand Up @@ -407,6 +408,10 @@ func (f *Fixer) Fix(specPath string) (*FixResult, error) {
return nil, fmt.Errorf("fixer: failed to parse specification: %w", err)
}

// Fix() owns the parsed document, so always mutate in place.
origMutable := f.MutableInput
f.MutableInput = true
defer func() { f.MutableInput = origMutable }()
return f.FixParsed(*parseResult)
}

Expand Down Expand Up @@ -445,12 +450,7 @@ func (f *Fixer) isFixEnabled(fixType FixType) bool {
if len(f.EnabledFixes) == 0 {
return true // if explicitly set to empty/nil, enable all fixes
}
for _, ft := range f.EnabledFixes {
if ft == fixType {
return true
}
}
return false
return slices.Contains(f.EnabledFixes, fixType)
}

// populateFixLocation fills in Line/Column/File from the SourceMap if available.
Expand Down
33 changes: 33 additions & 0 deletions fixer/fixer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,3 +601,36 @@ paths:
assert.Greater(t, len(pathItem.Get.Parameters), originalParamCount,
"Original document should be mutated when Fixer.MutableInput is true")
}

// TestFix_SetsInternalMutableInput verifies that Fix() internally sets
// MutableInput to skip the defensive copy and restores it afterward.
func TestFix_SetsInternalMutableInput(t *testing.T) {
t.Run("restores false to false", func(t *testing.T) {
f := New()
assert.False(t, f.MutableInput, "default should be false")

result, err := f.Fix("../testdata/bench/small-oas3.yaml")
require.NoError(t, err)
assert.True(t, result.Success)
assert.False(t, f.MutableInput, "MutableInput should be restored after Fix()")
})

t.Run("restores true to true", func(t *testing.T) {
f := New()
f.MutableInput = true

result, err := f.Fix("../testdata/bench/small-oas3.yaml")
require.NoError(t, err)
assert.True(t, result.Success)
assert.True(t, f.MutableInput, "MutableInput should be restored to original true value")
})

t.Run("untouched on parse error", func(t *testing.T) {
f := New()
f.MutableInput = true

_, err := f.Fix("does-not-exist.yaml")
require.Error(t, err)
assert.True(t, f.MutableInput, "MutableInput should be untouched when Fix() fails before save/restore")
})
}