panics with enable-all-rules: true: not enough arguments for cyclomatic rule, expected 1, got 0

[Dentrax]

Describe the bug

$ golangci-lint run ./... -v

INFO [config_reader] Config search paths: [./ /Users/furkan.turkal/go/src/foo /Users/furkan.turkal/go/src /Users/furkan.turkal/go /Users/furkan.turkal /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 27 linters: [asciicheck bodyclose depguard errcheck exportloopref gocritic gofmt gofumpt goimports gosec gosimple govet importas ineffassign misspell nilerr prealloc predeclared revive staticcheck stylecheck tparallel typecheck unconvert unparam unused whitespace]
INFO [loader] Go packages loading at mode 575 (exports_file|files|imports|types_sizes|compiled_files|deps|name) took 1.515643643s
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 11.18102ms
INFO [linters context] importas settings found, but no aliases listed. List aliases under alias: key.
panic: not enough arguments for cognitive-complexity rule, expected 1, got 0. Please check the rule's documentation

goroutine 7181 [running]:
github.com/mgechev/revive/rule.checkNumberOfArguments(...)
        github.com/mgechev/[email protected]/rule/utils.go:171
github.com/mgechev/revive/rule.(*CognitiveComplexityRule).configure(0xc009ed9e01?, {0x0?, 0xab00000004a7c3eb?, 0x5082680?})
        github.com/mgechev/[email protected]/rule/cognitive-complexity.go:22 +0x1ad
github.com/mgechev/revive/rule.(*CognitiveComplexityRule).Apply(0xc00106a220, 0xc0076bcfc0?, {0x0?, 0x14?, 0x0?})
        github.com/mgechev/[email protected]/rule/cognitive-complexity.go:35 +0x35
github.com/mgechev/revive/lint.(*File).lint(0xc0098c0080, {0xc009654000, 0x45, 0x80}, {0x0, 0x3fe999999999999a, {0x4e1063d, 0x7}, 0x1, 0xc0076bcfc0, ...}, ...)
        github.com/mgechev/[email protected]/lint/file.go:105 +0x19b
github.com/mgechev/revive/lint.(*Package).lint.func1(0xc003682180?)
        github.com/mgechev/[email protected]/lint/package.go:185 +0x85
created by github.com/mgechev/revive/lint.(*Package).lint
        github.com/mgechev/[email protected]/lint/package.go:184 +0xac

To Reproduce
Steps to reproduce the behavior:

  revive:
    enable-all-rules: true

Ref: https://golangci-lint.run/usage/linters/#revive

# flags

revive ...

revive:
    enable-all-rules: true

Expected behavior
It should work?

Logs
See the first section.

Desktop (please complete the following information):

• OS: [e.g. Ubuntu 18.04]
• Version of Go 1.19.1

golangci-lint has version 1.49.0 built from cc2d97f on 2022-08-23T12:03:20Z

Additional context
Add any other context about the problem here.

[chavacava]

Hi @Dentrax, thanks for filling the issue.

As is stated in the Configuration section of the README.md:
_ When enabling all rules you still need/can provide specific configurations for rules._

Rules like cognitive-complexity require to be parametrized to run.
The same section of the README provides a complete example of configuration with alll rules enabled.

(The idea of using default values for rules arguments was already discussed and discarded)

[Dentrax]

When enabling all rules you still need/can provide specific configurations for rules.

Sorry, I missed this! Isn't that a bit too complex for the end-user? This was my first try to run revive using golangci-lint and I got panic in first attempt. We can clarify the message instead of throw panic to provider much better UX.

Rules like cognitive-complexity require to be parametrized to run.

Shouldn't all rules have default values? The end user could not know the best value for a specific rule. For example, I could not know the correct parameter for the cognitive-complexity rule. Are there any known industry standard value? If not, why not we just skip?

The same section of the README provides a complete example of configuration with alll rules enabled.

As an end and fresh user of revive, I'd like to run everything by simply passing a flag. If just passing enableAllRules: true does not work as expected, then it breaks the principle of least surprise. We should think to deprecate this flag and enforce the user to pass all configurations individually.

(The idea of using default values for rules arguments was already discussed and discarded)

Can you please attach reference link? I'd like to read!

If the above statements have already been discussed and discarded, I can leave the use of the enable-all-rules flag as false by default, since I don't want to pass on additional configurations to contaminate my simple configuration.

Thanks! Please don't get me wrong, these are just my feedback.

[chavacava]

Sorry, I missed this! Isn't that a bit too complex for the end-user? This was my first try to run revive using golangci-lint and I got panic in first attempt. We can clarify the message instead of throw panic to provider much better UX.

panic: not enough arguments for cognitive-complexity rule, expected 1, got 0. Please check the rule's documentation

Do you think the message is not clear enough?

Shouldn't all rules have default values?

About default values, see below.

The end user could not know the best value for a specific rule.

End users of tools like revive are not lambda end-users but actual developpers.

As an end and fresh user of revive, I'd like to run everything by simply passing a flag.

The simplest way of running revive: revive ./...
Yes it does not "run everything" but it is simple.

If just passing enableAllRules: true does not work as expected, then it breaks the principle of least surprise.

Expectations are very personal. For example I'll be surprised if revive warns me about code lines longer than 80 characters (a potential default configuration for the rule line-length.

Anyway, enableAllRules does exactly that: it enables all available rules.

We should think to deprecate this flag and enforce the user to pass all configurations individually.

You can read this thread: #517

Can you please attach reference link? I'd like to read!

Related info on these threads but the main content is in #517

I can leave the use of the enable-all-rules flag as false

The flag is not mandatory, you can remove it from the configuration if you'll not use it

my simple configuration

a simple golangci-lint configuration, interesting ;)

Thanks! Please don't get me wrong, these are just my feedback.

Thank you! All feedbacks are welcome

[AlexBasile123]

Looking to add golangci-lint to a new project. (Hopefully more to come) Trying to figure out how to disable just one rule (i.e. var-naming) from revive.
Read through the posts and docs, but still don't have it working.

1. enableAllRules is not doing the trick. i.e. Not showing the other rule issues
2. enable-all-rules still panics on the args. I believe the arg is a list of whitelist & blacklists: https://github.com/mgechev/revive#configurable-rules
   Incidentally, this doc is broken: https://revive.run/RULES_DESCRIPTIONS.md#var-naming

Kindly point me to doc(s) you're referring to or even better if you could please explain why the config below is causing the panic.

Much appreciated!

linters-settings:
      revive:
          ignore-generated-header: true
          enable-all-rules: true
          #enableAllRules: true
          rules:
            - name: var-naming
              disabled: false
              arguments: [["ID"], ["VM"]]

[chavacava]

Hi @AlexBasile123, the Configuration section of the README.md provides the information you are looking for:

By default revive will enable only the linting rules that are named in the configuration file.
For example, the previous configuration file makes revive to enable only cyclomatic and package-comments linting rules.

To enable all available rules you need to add:

enableAllRules = true

This will enable all available rules no matter of what rules are named in the configuration file.

To disable a rule, you simply mark it as disabled in the configuration.
For example:

[rule.line-length-limit]
    Disabled = true

When enabling all rules you still need/can provide specific configurations for rules.
The following files is an example configuration were all rules are enabled, with exception to those that are explicitly disabled, and some rules are configured with particular arguments:

severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

# Enable all available rules
enableAllRules = true

# Disabled rules
[rule.blank-imports]
    Disabled = true
[rule.file-header]
    Disabled = true
[rule.max-public-structs]
    Disabled = true
[rule.line-length-limit]
    Disabled = true
[rule.function-length]
    Disabled = true
[rule.banned-characters]
    Disabled = true

# Rule tuning
[rule.argument-limit]
    Arguments = [5]
[rule.cyclomatic]
    Arguments = [10]
[rule.cognitive-complexity]
    Arguments = [7]
[rule.function-result-limit]
    Arguments = [3]
[rule.error-strings]
    Arguments = ["mypackage.Error"]

In your case, if you are searching to disable var-namming you need to set Disabled: true (and not to false as in the configuration you provided)
Notice that when you enable all rules, you need to provide mandatory configuration for some rules (see the example of configuration above this paragraph) Not doing so, will cause revive to panic.

[AlexBasile123]

Thanks Chavacava for the quick response!

Before pasting the config I forgot to set disabled back to true.

By specifying any rules, whether disabled or not, it only runs those rules. Changing enableAllRules to true doesn't have any affect. Meaning, it will respect just the rules that were listed.

I'll try setting the mandatory arguments for all rules when using enable-all-rules. Would've been nice not to and just keep the current defaults as when you don't specify any rules.
According to this link, there doesn't seem to be too many mandatory rules: https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#var-naming
If there's a better doc please let me know.

All in all, sounds like listing out the rules (at least the ones w/ mandatory args) is the way to go.

I appreciate your feedback.

[chavacava]

Hello, I've made a PR (#830) that will set default configuration to some rules in order to do not panic when running revive with enableAllrules = true
Of course these defaults are very opinionated thus your comments and remarks are all welcome.