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

Make ktlint follow @Suppress/@SuppressWarning annotations #765

Closed
sjcqs opened this issue Jun 4, 2020 · 3 comments · Fixed by #1463
Closed

Make ktlint follow @Suppress/@SuppressWarning annotations #765

sjcqs opened this issue Jun 4, 2020 · 3 comments · Fixed by #1463
Milestone

Comments

@sjcqs
Copy link

sjcqs commented Jun 4, 2020

I just read #263 thinking it would also provide a way to use @Suppress/@SuppressWarning for the other rules but it's only for RemoveCurlyBracesFromTemplate.

I was going to make a PR to fix the issue but I'd like to discuss possible implementations.

Currently

Annotations are read and the rule name inside is then mapped to the ktlint rule-name.
(RemoveCurlyBracesFromTemplate is the rule name from Jetbrains Kotlin code inspections for Intellij)

private val suppressAnnotationRuleMap = mapOf(
"RemoveCurlyBracesFromTemplate" to "string-template"
)
private val suppressAnnotations = setOf("Suppress", "SuppressWarnings")
/**
* Creates [SuppressionHint] from annotations of given [psi]
* Returns null if no [targetAnnotations] present or no mapping exists
* between annotations' values and ktlint rules
*/
private fun createSuppressionHintFromAnnotations(
psi: KtAnnotated,
targetAnnotations: Collection<String>,
annotationValueToRuleMapping: Map<String, String>
): SuppressionHint? {
return psi.annotationEntries
.filter {
it.calleeExpression?.constructorReferenceExpression
?.getReferencedName() in targetAnnotations
}.flatMap(KtAnnotationEntry::getValueArguments)
.mapNotNull { it.getArgumentExpression()?.text?.removeSurrounding("\"") }
.mapNotNull(annotationValueToRuleMapping::get)
.let { suppressedRules ->
if (suppressedRules.isNotEmpty()) {
SuppressionHint(
IntRange(psi.startOffset, psi.endOffset),
suppressedRules.toSet()
)
} else {
null
}
}
}
}
}

To support other rules

  1. Use ktlint rule name inside suppress annotations
  2. Use Jetbrains rules name inside suppress annotations and map it to ktlint rule name (if it exists, otherwise create a PascalCase name).

In the second case, we could either keep the suppressAnnotationRuleMap and just add every rule inside of it, which is not SOLID since every time a rule is added, this map needs to change and it's not really the concern of Ktlint object to know it's rules' name...
The other solution would be to add an attribute suppressRuleName to Rule so that every rule will have it's own.

The argument of using @Suppress along with Kotlin code inspections name would be to have both your favorite editor and ktlint inspection support it.

@Tapchicoma
Copy link
Collaborator

suppressAnnotationRuleMap is a temp workaround until better solution. Ideally rules themselves should provide such information, rather then collect it inside KtLint.kt.

Let me think how to do it properly and come back to you.

@sjcqs
Copy link
Author

sjcqs commented Jul 2, 2020

Hey @Tapchicoma did you have so spare time to think about this ?

@ghost
Copy link

ghost commented Mar 15, 2022

+1

paul-dingemans pushed a commit to paul-dingemans/ktlint that referenced this issue May 8, 2022
…refixed by `ktlint:`. Standard rules can be suppressed using `@Suppress("ktlint:max-line-length")` while non-standard rules need to be suppressed with `@Suppress("ktlint:experimental:trailing-comma")` Also allow to use `@Suppress("ktlint")` to suppress all rules.

Closes pinterest#765
Closes pinterest#1458
@paul-dingemans paul-dingemans added this to the 0.46.0 milestone May 8, 2022
paul-dingemans added a commit that referenced this issue May 20, 2022
…refixed by `ktlint:`

Standard rules can be suppressed using `@Suppress("ktlint:max-line-length")` while non-standard rules need to be suppressed with `@Suppress("ktlint:experimental:trailing-comma")` Also allow to use `@Suppress("ktlint")` to suppress all rules. (#1463)

Closes #765
Closes #1458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants