Skip to content

Add fixer for prefer-equals-comparison rule#1790

Merged
charlieegan3 merged 3 commits intoopen-policy-agent:mainfrom
SeanLedford:s_ledford/prefer-equals-comparison-fixer
Dec 8, 2025
Merged

Add fixer for prefer-equals-comparison rule#1790
charlieegan3 merged 3 commits intoopen-policy-agent:mainfrom
SeanLedford:s_ledford/prefer-equals-comparison-fixer

Conversation

@SeanLedford
Copy link
Copy Markdown
Contributor

@SeanLedford SeanLedford commented Dec 2, 2025

PR to add fixer for the prefer-equals-comparison rule, as well as adequate coverage and docs updates. Done to address issue #1734.

@SeanLedford SeanLedford force-pushed the s_ledford/prefer-equals-comparison-fixer branch from 6242c6e to 068313e Compare December 3, 2025 22:53
return
}

if tc.fixExpected && fixResults[0].Contents != tc.contentAfterFix {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to look into using cmp.Diff, that can be helpful for comparing strings. we use it in some other tests.

Filename: "test.rego",
Contents: "package test\n\nlong := true\n\nlonger = true\nlongest = false\n",
},
contentAfterFix: "package test\n\nlong := true\n\nlonger == true\nlongest == false\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for some of these tests we might use a raw string instead.

Suggested change
contentAfterFix: "package test\n\nlong := true\n\nlonger == true\nlongest == false\n",
contentAfterFix: `package test
long := true
longer == true
longest == false
`,

Comment thread pkg/fixer/fixes/fixes.go
&NoWhitespaceComment{},
&DirectoryPackageMismatch{},
&NonRawRegexPattern{},
&PreferEqualsComparison{},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you run

go run main.go lint foo/wow.rego
go run main.go fix --force foo/wow.rego

?

There seems to be a slight difference in how the locations are matched which is causing the fix not to fire.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, addressed! Wondering how we feel about updating the rule itself to provide the exact column location of the violation 🤔, the use-assignment-operator rule does something like that it looks like.

Signed-off-by: Sean Ledford <s_ledford@apple.com>
Signed-off-by: Sean Ledford <s_ledford@apple.com>
Signed-off-by: Sean Ledford <s_ledford@apple.com>
@SeanLedford SeanLedford force-pushed the s_ledford/prefer-equals-comparison-fixer branch from 068313e to c24c127 Compare December 5, 2025 00:13
@SeanLedford SeanLedford marked this pull request as ready for review December 5, 2025 00:19
Copy link
Copy Markdown
Contributor

@charlieegan3 charlieegan3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me Sean! thanks for this

@charlieegan3 charlieegan3 merged commit f22c2aa into open-policy-agent:main Dec 8, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants