rule: allow lowercased and kebab-cased options #1272

alexandear · 2025-03-15T18:46:31Z

This PR allows rule options to be specified in lowercase and kebab case. Previously, the context-as-argument rule accepts camelCased argument allowTypesBefore. Now, it also accepts kebab-cased allow-types-before and the fully lowercased allowtypesbefore option.

This PR addresses the following issues:

Allows users to specify options in lowercase (see Raw map configuration and case sensitivity golangci/golangci-lint#3280).
Ensures consistent casing for all revive rule options. For example, most rules use camelCase for options, but context-keys-type uses kebab-cased options like call-chain, method-call, and immediate-recover.
Aligns revive rule options with those of other Go linters, which commonly use kebab-case or lowercase options. See .golangci.reference.yml.

I've refactored early-return, indent-error-flow, and superfluous-else rules by adding the Configure method.

I’ve added tests to ensure this PR does not introduce regressions.

Also, updated documentation.

To simplify the review process, I have broken the changes into separate commits, each representing a distinct logical update.

ccoVeille

… but … 😅😂

rule/add_constant.go

ccoVeille

A lot of nitpicking from me.
Sent with love anyway.

I appreciate a lot the changes you made.

My feedbacks could also be addressed in another PR maybe.

RULES_DESCRIPTIONS.md

rule/add_constant_test.go

ccoVeille · 2025-03-17T13:59:17Z

rule/add_constant.go

@@ -241,7 +241,7 @@ func (r *AddConstantRule) Configure(arguments lint.Arguments) error {
 				return fmt.Errorf("invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)
 			}
 			r.strLitLimit = limit
-		case "ignoreFuncs":
+		case normalizeRuleOption("ignoreFuncs"):
 			excludes, ok := v.(string)
 			if !ok {
 				return fmt.Errorf("invalid argument to the ignoreFuncs parameter of add-constant rule, string expected. Got '%v' (%T)", v, v)


Here and everywhere, I would use the provided parameter with its exact syntax

Suggested change

return fmt.Errorf("invalid argument to the ignoreFuncs parameter of add-constant rule, string expected. Got '%v' (%T)", v, v)

return fmt.Errorf("invalid argument to the %s parameter of add-constant rule, string expected. Got '%v' (%T)", k, v, v)

So if someone uses :

ignoreFuncs the error message is about ignoreFuncs

ignore-funcs the error message is also clearer

It's not about updating an error message that can be caught with golangci-lint issues filtering, so we can update it.

ccoVeille · 2025-03-17T14:01:32Z

rule/add_constant.go

@@ -241,7 +241,7 @@ func (r *AddConstantRule) Configure(arguments lint.Arguments) error {
 				return fmt.Errorf("invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)


Here (and everywhere else maybe), we could/should use a better error message like the one used for ignoreFuncs

Suggested change

return fmt.Errorf("invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)

return fmt.Errorf("invalid argument to the maxLit parameter of add-constant rule, expecting string representation of an integer. Got '%v'", v)

But I would encourage you using what I suggested here

#1272 (comment)

Suggested change

return fmt.Errorf("invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)

return fmt.Errorf("invalid argument to the %v parameter of add-constant rule, expecting string representation of an integer. Got '%v'", k, v)

ccoVeille · 2025-03-17T14:22:19Z

rule/unused_receiver.go

+		// Arguments = [{allowRegex="^_"}]
+		allowRegexStr, ok := v.(string)
+		if !ok {
+			return fmt.Errorf("error configuring [unused-receiver] rule: allowRegex is not string but [%T]", v)


Here we have another style, brackets are used

So we have plain text, single quote, double quote, brackets...

rule/utils.go

rule/indent_error_flow.go

test/add_constant_test.go

ccoVeille · 2025-03-17T14:33:44Z

rule/var_naming.go

@@ -279,7 +285,7 @@ func getList(arg any, argName string) ([]string, error) {
 	for _, v := range args {
 		val, ok := v.(string)
 		if !ok {
-			return nil, fmt.Errorf("invalid %s values of the var-naming rule. Expecting slice of strings but got element of type %T", val, arg)
+			return nil, fmt.Errorf("invalid %v values of the var-naming rule. Expecting slice of strings but got element of type %T", v, arg)


Here another oddity, that can be seen in the unit test file you added

wantErr: errors.New("invalid argument to the var-naming rule. Expecting a allowlist of type slice with initialisms, got int"), (...) wantErr: errors.New("invalid 123 values of the var-naming rule. Expecting slice of strings but got element of type []interface {}"),

The first error message mention the name of the argument, the second doesn't

Maybe something like this, that remove the "a", because "a allowList" sounds bad.

Suggested change

return nil, fmt.Errorf("invalid %v values of the var-naming rule. Expecting slice of strings but got element of type %T", v, arg)

return nil, fmt.Errorf("invalid %v values of the var-naming rule. Expecting '%v' parameter to be a slice of strings but got element of type %T", argName, v, arg)

ccoVeille · 2025-03-17T15:10:24Z

Also, one question,

I saw you worked on golangci-lint too

docs: add arguments to a few revive rules golangci/golangci-lint#5562

do you plan to update the exanples in json schema too golangci-lint reference.yaml ?

https://github.com/golangci/golangci-lint/blob/main/jsonschema%2Fgolangci.next.jsonschema.json#L2826-L2849

Also, how much work do you think it could be to add for each rule the list of parameter it supports.

alexandear · 2025-03-17T15:40:17Z

My feedbacks could also be addressed in another PR maybe.

Thanks for the review! I’ve fixed half of the comments. The other comments are about making error messages consistent and should be addressed in a separate PR to avoid overcomplicating this one. In this PR, I tried to leave them unchanged.

I saw you worked on golangci-lint too

docs: add arguments to a few revive rules golangci/golangci-lint#5562

The PR golangci/golangci-lint#5562 adds missing parameters to the golangci-lint documantation. We will create a PR to switch from camelCase to kebab-case after the new revive version is released with this change.

do you plan to update the exanples in json schema too golangci-lint reference.yaml ?

No, I'm not. Ludovic usually updates the schema.

Co-authored-by: ccoVeille <[email protected]>

chavacava · 2025-03-28T08:35:01Z

Thanks @alexandear and @ccoVeille !

* rule: tests for Configure with named options; fix errors * rule: refactor and add tests for ifelse rules * rule: allow lowercased and kebab-cased options * test: update integration tests with lowercased params * docs: update rules descriptions * rule: simplify Configure implementation with one option * gofmt and fix lint * review: add isRuleOption, update grammar in doc, simplify regex Co-authored-by: ccoVeille <[email protected]> --------- Co-authored-by: ccoVeille <[email protected]>

alexandear marked this pull request as draft March 15, 2025 18:46

alexandear force-pushed the lowercase-rule-arguments branch 2 times, most recently from abcc9f3 to 79142d4 Compare March 15, 2025 19:30

alexandear changed the title ~~rule: allow lowercased rule arguments~~ rule: allow lowercased and dashed rule arguments Mar 16, 2025

alexandear changed the title ~~rule: allow lowercased and dashed rule arguments~~ rule: allow lowercased and dashed options Mar 16, 2025

alexandear force-pushed the lowercase-rule-arguments branch from 1be8855 to 8bd4a2f Compare March 16, 2025 09:34

alexandear changed the title ~~rule: allow lowercased and dashed options~~ rule: allow lowercased and kebab-cased options Mar 16, 2025

ccoVeille reviewed Mar 16, 2025

View reviewed changes

rule/add_constant.go Outdated Show resolved Hide resolved

alexandear force-pushed the lowercase-rule-arguments branch 2 times, most recently from 7fc1ea6 to 7b6e1fe Compare March 17, 2025 13:15

alexandear added 7 commits March 17, 2025 15:17

rule: tests for Configure with named options; fix errors

5aaeeb6

rule: refactor and add tests for ifelse rules

f1a91a6

rule: allow lowercased and kebab-cased options

1ad4072

test: update integration tests with lowercased params

de7af97

docs: update rules descriptions

6f9b16e

rule: simplify Configure implementation with one option

6a14548

gofmt and fix lint

dfead4b

alexandear force-pushed the lowercase-rule-arguments branch from 7b6e1fe to dfead4b Compare March 17, 2025 13:18

alexandear requested a review from ccoVeille March 17, 2025 13:18

alexandear marked this pull request as ready for review March 17, 2025 13:18

ccoVeille reviewed Mar 17, 2025

View reviewed changes

ccoVeille approved these changes Mar 17, 2025

View reviewed changes

alexandear force-pushed the lowercase-rule-arguments branch from 0c9891c to 0a85f9e Compare March 17, 2025 15:43

review: add isRuleOption, update grammar in doc, simplify regex

200264f

Co-authored-by: ccoVeille <[email protected]>

alexandear force-pushed the lowercase-rule-arguments branch from 6d99aa1 to 200264f Compare March 17, 2025 15:45

chavacava merged commit 9f5f957 into mgechev:master Mar 28, 2025
8 checks passed

alexandear deleted the lowercase-rule-arguments branch March 28, 2025 09:23

This was referenced Mar 28, 2025

REQUEST: Maintainer membership for alexandear #1283

Closed

Consistent error messages #1284

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

rule: allow lowercased and kebab-cased options #1272

rule: allow lowercased and kebab-cased options #1272

Uh oh!

alexandear commented Mar 15, 2025 •

edited

Loading

Uh oh!

ccoVeille left a comment

Uh oh!

Uh oh!

ccoVeille left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ccoVeille Mar 17, 2025

Uh oh!

ccoVeille Mar 17, 2025

Uh oh!

ccoVeille Mar 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ccoVeille Mar 17, 2025

Uh oh!

ccoVeille commented Mar 17, 2025 •

edited

Loading

Uh oh!

alexandear commented Mar 17, 2025

Uh oh!

Uh oh!

chavacava commented Mar 28, 2025

Uh oh!

Uh oh!

	return fmt.Errorf("invalid argument to the ignoreFuncs parameter of add-constant rule, string expected. Got '%v' (%T)", v, v)
	return fmt.Errorf("invalid argument to the %s parameter of add-constant rule, string expected. Got '%v' (%T)", k, v, v)

		@@ -241,7 +241,7 @@ func (r *AddConstantRule) Configure(arguments lint.Arguments) error {
		return fmt.Errorf("invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)

	return nil, fmt.Errorf("invalid %v values of the var-naming rule. Expecting slice of strings but got element of type %T", v, arg)
	return nil, fmt.Errorf("invalid %v values of the var-naming rule. Expecting '%v' parameter to be a slice of strings but got element of type %T", argName, v, arg)

rule: allow lowercased and kebab-cased options #1272

rule: allow lowercased and kebab-cased options #1272

Uh oh!

Conversation

alexandear commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccoVeille left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ccoVeille left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ccoVeille Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

ccoVeille Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

ccoVeille Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ccoVeille Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

ccoVeille commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexandear commented Mar 17, 2025

Uh oh!

Uh oh!

chavacava commented Mar 28, 2025

Uh oh!

Uh oh!

alexandear commented Mar 15, 2025 •

edited

Loading

ccoVeille commented Mar 17, 2025 •

edited

Loading