Conversation
…e rule Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix false positive for openapi plugin snake_case config keys
fix(modernize): camelcase-config rule false-positives on schema-defined snake_case keys
Mar 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the modernize CLI’s camelcase-config rule to avoid false positives when a module’s schema intentionally defines snake_case config keys (e.g., openapi), preventing suggestions that would break valid configs.
Changes:
- Teach
camelcase-configto consult the module schema registry and exempt schema-declared config keys fromsnake_casefindings. - Add tests confirming schema-defined
snake_casekeys foropenapiare not flagged. - Add a regression test ensuring unknown module types still have
snake_casekeys flagged.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| modernize/rules.go | Exempts schema-declared config keys (per module type) from the snake_case check to prevent invalid modernization advice. |
| cmd/wfctl/modernize_test.go | Adds coverage for the new exemption behavior and ensures unknown module types retain prior behavior. |
Comments suppressed due to low confidence (1)
modernize/rules.go:637
- The rule description says the engine "requires camelCase", but the rule now explicitly allows schema-defined snake_case keys. Consider adjusting the description to avoid implying snake_case is always invalid (e.g., "typically requires camelCase" or "except where the module schema defines otherwise").
ID: "camelcase-config",
Description: "Detect snake_case config field names (engine requires camelCase)",
Severity: "warning",
Comment on lines
+525
to
+530
| doc := parseTestYAML(t, input) | ||
| findings := rule.Check(doc, []byte(input)) | ||
| if len(findings) == 0 { | ||
| t.Fatal("expected findings for snake_case config keys on unknown module type") | ||
| } | ||
| } |
There was a problem hiding this comment.
This test only asserts that some finding is produced. To better cover the intended behavior and catch regressions (e.g., wrong key reported), assert that at least one finding message mentions snake_key (and optionally the module name/type) instead of only checking len(findings) > 0.
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Contributor
|
@copilot apply changes based on the comments in this thread Also fix the failing linter |
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Contributor
Author
Applied in b0249f4:
|
Contributor
|
@copilot Fix the failing linter |
Contributor
Author
The linter is already passing on the current HEAD ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
camelcase-configrule flagged snake_case config keys that are intentionally snake_case because the module's own schema defines them that way (e.g.openapi'sspec_file,register_routes,swagger_ui,max_body_bytes). Suggesting camelCase for these keys would break the config.Changes
modernize/rules.go:camelCaseConfigRule()now instantiatesschema.NewModuleSchemaRegistry()once at rule-creation time. For each module node, it resolves thetypefield and builds a set of schema-defined key names — any key matching a schema-declared field is exempt from the snake_case check.cmd/wfctl/modernize_test.go: Two new tests:openapi) produce zero findingsExample — no longer flagged
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.