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

Setting 'warning' in SeverityLevelsConfig should unset 'error' #409

Closed
mgrebenets opened this issue Jan 24, 2016 · 8 comments
Closed

Setting 'warning' in SeverityLevelsConfig should unset 'error' #409

mgrebenets opened this issue Jan 24, 2016 · 8 comments
Assignees
Labels
enhancement Ideas for improvements of existing features and rules.

Comments

@mgrebenets
Copy link
Contributor

I am getting false positive error messages from swiftlint version 0.6.0.

Here's a picture that describes it the best.
image

.swiftlint.yml looks like this:

disabled_rules: # Rule identifiers to exclude from running
  - nesting
  - todo
excluded: # Paths to ignore during linting. Overridden by `included`.
  - Carthage
  - Pods
file_length: 600
line_length: 250
type_body_length: 500
function_body_length: 200
variable_name_max_length:
  - 48 # warning
  - 64 # error

The error stays no matter what, even if I bump some values, for example, increasing type_body_length to 700:
image

@scottrhoyt
Copy link
Contributor

I see what is going on here. TypeBodyLength has both warning and error levels. While you are successfully raising the warning level to 500, the error level is remaining the default of 350. An immediate workaround would be to specify this in your .swiftlint.yml:

type_body_length:
  warning: 400
  error: 700 # something higher than 400

@jpsim we should consider if we should allow warning levels to be set higher than error levels. I see validity in each approach.

@jpsim
Copy link
Collaborator

jpsim commented Jan 25, 2016

@scottrhoyt we could consider it a configuration error (e.g. throw ConfigurationError) to set a warning to be "less strict" than the error value (in this case, higher). Or maybe it'd be preferable to erase the error value when setting a warning value?

@scottrhoyt
Copy link
Contributor

@jpsim I think I like the latter route better because the former requires that we expose the knowledge of rules with increasing warning/error thresholds (e.g. FileLengthRule) and those with decreasing thresholds (e.g minLength for TypeNameRule). It also has the value of making .swiftlint.yml more deterministic.

@jpsim jpsim changed the title False positive error messages on line and type body length Setting 'warning' in SeverityLevelsConfig should unset 'error' Jan 25, 2016
@jpsim jpsim added the enhancement Ideas for improvements of existing features and rules. label Jan 25, 2016
@jpsim
Copy link
Collaborator

jpsim commented Jan 25, 2016

Agreed. I've updated this issue title to better track that.

@robinkunde
Copy link
Contributor

@scottrhoyt I came across the same problem today and prepared a pull request (#431) similar to the solution you agreed on before I found this issue.

@scottrhoyt
Copy link
Contributor

Thanks for the contribution @robinkunde, the SwiftLint community appreciates it! Looking forward to checking it out.

@scottrhoyt
Copy link
Contributor

@jpsim this is closed by #438, correct?

@jpsim
Copy link
Collaborator

jpsim commented Jan 29, 2016

Yup! Thanks for being ever vigilant!

@jpsim jpsim closed this as completed Jan 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules.
Projects
None yet
Development

No branches or pull requests

4 participants