Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows inversing the semantics of string-format rule configurations #765

Merged
merged 3 commits into from
Oct 24, 2022

Conversation

chavacava
Copy link
Collaborator

Adds a syntactic mean to negate the regular expressions in the configuration to pass from a "accept if match" to "fail if match" mode.

Now, using a ! the configuration meaning is inverted

with "/^[A-Z].$/" the rule will accept strings starting with capital letters
with "!/^[A-Z].
$/" the rule will a fail on strings starting with capital letters

Closes #762

failure = r.errorMessage
} else {
failure = fmt.Sprintf("string literal doesn't match user defined regex /%s/", r.regexp.String())
switch r.negated {
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we can reduce nesting if we use if/else rather than switch:

if r.negated {
  // ...
}
// else here

failure = fmt.Sprintf("string literal doesn't match user defined regex /%s/", r.regexp.String())
switch r.negated {
case false:
// Fail if the string doesn't match the user's regex
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
// Fail if the string doesn't match the user's regex
// Fail if the string does NOT match the user's regex

Copy link
Owner

Choose a reason for hiding this comment

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

To make it easier to read :)

Copy link
Owner

@mgechev mgechev left a comment

Choose a reason for hiding this comment

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

LGTM. Left few nits, feel free to consider them and merge.

@chavacava chavacava merged commit 32a0cb8 into master Oct 24, 2022
@chavacava chavacava deleted the feature/762 branch December 3, 2022 09:17
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.

string-format: Missing possibility to consider matches as linting errors
2 participants