crystal-ameba / ameba

A static code analysis tool for Crystal
https://crystal-ameba.github.io
MIT License
515 stars 35 forks source link

`--only` CLI flag appears to ignore excluded files within config #396

Closed syeopite closed 2 months ago

syeopite commented 1 year ago

In my .ameba.yml I have this file excluded for the CyclomaticComplexity rule.

Metrics/CyclomaticComplexity:
  Excluded:
  - src/invidious/videos/parser.cr

But when running ameba with the --only option ./lib/ameba/bin/ameba --only Metrics/CyclomaticComplexity

It seems like Ameba still complains about the issue

src/invidious/videos/parser.cr:157:5
[W] Metrics/CyclomaticComplexity: Cyclomatic complexity too high [58/10]
> def parse_video_info(video_id : String, player_response : Hash(String, JSON::Any)) : Hash(String, JSON::Any)

Ameba is able to ignore the file fine when running without --only

It makes sense for options like --all to ignore rules but I don't think the same applies for --only. Is this behavior intended?

veelenga commented 1 year ago

That's a bug. Thanks for reporting.

syeopite commented 3 months ago

Hello, if you don't mind me asking, can I inquire about the status of this bug? I saw that #398 was closed back in January.

veelenga commented 3 months ago

@syeopite Hey, sorry for the delayed response. So, it looks like that behavior was intentional because --only option allows testing specific rules regardless of the config. Even the generated comment in the config (2nd row) says that it is possible to test the rule without changing the config:

# Problems found: 4
# Run `ameba --only Lint/Typos` for details
Lint/Typos:
  Description: Reports typos found in source files
  FailOnError: false
  Excluded:
  - spec/ameba/rule/lint/typos_spec.cr
  Enabled: true
  Severity: Warning

In #398 I was thinking about changing to comment to something like Remove the section below and run but not sure if that is really convenient. So I would prefer leaving it as is.

@Sija do you have any suggestions here?

Sija commented 3 weeks ago

I could see an enhancement in a way of marking issues from excluded paths.

syeopite commented 2 weeks ago

I think it'll be useful to have a separate --only option that does adhere to the config