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

Baseline ignoring MaxLineLength: on 1.0.1 #1906

Closed
nicodn opened this issue Sep 9, 2019 · 11 comments · Fixed by #2083
Closed

Baseline ignoring MaxLineLength: on 1.0.1 #1906

nicodn opened this issue Sep 9, 2019 · 11 comments · Fixed by #2083

Comments

@nicodn
Copy link

nicodn commented Sep 9, 2019

Expected Behavior

I upgraded from 1.0.0-RC14 to 1.0.1 and my baseline file stopped working.

Observed Behavior

I had a baseline file like so:

<?xml version="1.0" ?>
<SmellBaseline>
  <Whitelist>
    <ID>MaximumLineLength:</ID>
    <!-- other rules with specific signatures -->
  </Whitelist>
</SmellBaseline>

After upgrading I get over 200 issues failed with MaxLineLength. Even if I add <ID>MaxLineLength:</ID> to the baseline file it fails.

Steps to Reproduce

Relevant build.gradle

plugins {
id "io.gitlab.arturbosch.detekt" version "1.0.1"
}

dependencies {
detektPlugins "io.gitlab.arturbosch.detekt:detekt-formatting:1.0.1"
}

detekt {
    toolVersion = "1.0.1"
    input = files("src/main/kotlin", "src/test/kotlin")
    filters = ".*/resources/.*,.*/build/.*"
    baseline = file(".config/detektBaseline.xml")
}

Context

If I regenerate the baseline it adds each of the occurrences of MaxLineLength with it's own signature and at this point it works but the baseline file becomes unmaintainable.

Your Environment

  • Version of detekt used: 1.0.1
  • Version of Gradle used (if applicable): 5.2.1
  • Operating System and version: macOS 10.14.5
  • Link to your project (if it's a public repository):
@3flex
Copy link
Member

3flex commented Sep 10, 2019

If you want to ignore all instances of MaximumLineLength you could disable the rule in the YAML config file instead.

I don't know the history of baseline file support so I'm not sure whether adding items to the baseline like that was ever actually supported, but I might be wrong. The docs only describe adding the entire signature to the baseline.

@schalkms
Copy link
Member

schalkms commented Sep 10, 2019

@nicodn Please be aware that there are two rules which basically do the same.
MaxLineLength is the detekt native rule. This one provides more configuration options.
MaximumLineLength is the wrapped ktlint rule.
So you need to choose between one of the two.

In your code only MaximumLineLength is included in your baseline file. MaxLineLength is missing. That's why it's reporting over 200 issues in your code.

HTH

@nicodn
Copy link
Author

nicodn commented Sep 10, 2019

Hi @schalkms , thanks for your reply.

I tried adding <ID>MaxLineLength:</ID> to the whitelist in the baseline file but it still gives the 200+ errors.

It's strange because on 1.0.0-RC14 it never reported any issues with MaxLineLength.

@arturbosch
Copy link
Member

Hi, the baseline does not support the formatting ruleset (yet).
Also an ID entry looks like this NewLineAtEndOfFile:PluginVersion.kt$io.gitlab.arturbosch.detekt.PluginVersion.kt</ID>.
It is not only the rule id but the signature of a code element. In this case it is the whole file itself.
Try running detektBaseline and see what happens.

Additionally we could indeed support suppressing formatting findings via baseline like:
MaximumLineLengthPluginVersion.kt$io.gitlab.arturbosch.detekt.PluginVersion.kt#[line].[column]</ID>.

@schalkms
Copy link
Member

@arturbosch What do you mean by that?
Currently, the baseline feature doesn't support rules in the formatting rule set.
We would need to adapt detekt's codebase in order to support this feature.
Am I right?

@arturbosch
Copy link
Member

Yes, the reporting logic (Entity, Location and stuff) could check if this finding is reported on a KtFile and add the line number to the entities signature. I'm not sure if any non KtLint rules also suffer from this. Need to check on WE.

@arturbosch arturbosch added this to the 1.1.0 milestone Sep 12, 2019
@arturbosch arturbosch added this to Backlog in Roadmap Sep 16, 2019
@arturbosch arturbosch modified the milestones: 1.1.0, 1.2.0 Sep 17, 2019
@arturbosch arturbosch removed this from Backlog in Roadmap Sep 17, 2019
@blazeroni
Copy link

@arturbosch Some additional info. It looks like the formatting of the MaxLineLength rule changed for certain elements. I recently upgraded to 1.0.1 from RC14 and saw a lot of errors that were already in the baseline. I reran the baseline and saw that rules previously formatted like this:

<ID>MaxLineLength:FileName.kt$ClassName$private val fieldName = {code}</ID>
<ID>MaxLineLength:FileName.kt$ClassName$override fun method(...) {</ID>

became this after the baseline was regenerated:

<ID>MaxLineLength:FileName.kt$ClassName$private</ID>
<ID>MaxLineLength:FileName.kt$ClassName$override</ID>

They are no longer uniquely identified and apparently no longer match older baselines.

@arturbosch
Copy link
Member

I could not reproduce this issue, my tested MaxLineLength findings have a correct signature.
Could you test it with 1.1.1 again?
I've implemented a way to suppress formatting issues in the link PR.

@arturbosch
Copy link
Member

I could reproduce this on the sonar-kotlin repository.
The signature is right and contains the whole line but is not considered when filtering findings.

@arturbosch arturbosch reopened this Nov 14, 2019
Roadmap automation moved this from Done to Next Nov 14, 2019
@arturbosch arturbosch modified the milestones: 1.2.0, 1.2.1 Nov 16, 2019
@arturbosch arturbosch moved this from Next to Backlog in Roadmap Nov 16, 2019
@arturbosch arturbosch removed this from the 1.2.1 milestone Nov 25, 2019
@arturbosch arturbosch removed this from Backlog in Roadmap Mar 15, 2020
@schalkms
Copy link
Member

@arturbosch Is this reproducible after 6b21237 in your scenario?

@arturbosch
Copy link
Member

@schalkms baseline suppressing works as expected in current master ^^

Bugs and False Positives automation moved this from Next to Done Apr 14, 2020
@arturbosch arturbosch added this to the 1.8.0 milestone Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

5 participants