-
Notifications
You must be signed in to change notification settings - Fork 504
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
Add angle bracket whitespace rule #587
Conversation
The new angle bracket rule will live in the experimental rule set along with all the other new rules, so this commit refactors the associated tests to also live under the experimental package
Adds a rule to lint against white space before and after angle brackets in type arguments. This includes all of the following: Map< String, Int> Map<String, Int > Map <String, Int> In all of the above cases, the desired format is Map<String, Int> Specifically this rule does not affect generics formatting of the type fun myFun<T>() as this is already implemented in a separate rule.
import com.pinterest.ktlint.core.ast.prevLeaf | ||
import org.jetbrains.kotlin.com.intellij.lang.ASTNode | ||
|
||
class SpacingAroundAngleBracketsRule : Rule("angle-brackets-rule") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: please leave a small comment about what this rule does.
@Tapchicoma want to take a look at this one? Feel free to merge it in if it looks good to you. |
if (openingBracketChild != null) { | ||
// Check for rogue spacing before an opening bracket: Map <String, Int> | ||
val beforeAngleBracket = openingBracketChild.prevLeaf() | ||
if (beforeAngleBracket != null && beforeAngleBracket.elementType == WHITE_SPACE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simplified to beforeAngleBracket?.elementType == WHITE_SPACE
, also in other such ifs
. If you don't have special conventions for that, of course.
Just popped into my mind, what if the argument list is huge and we want to split the types per line? E.g., this would work with the current implementation:
But this would already fail:
So if you want to allow the latter case, then we need to add another check for a newline. |
@rom4ek added valid point about covering writing type parameters per line. I've checked kotlin and android style guides and don't found anything there. IDEA allows following formatting: class AngleTest<
S,
G,
Z,
B
> {
} Also would be nice to add tests for nested angle brakets cases, for example: class AngleTest<B : Map<Int, List<String>>> |
Sounds good on all counts. Will update. |
@christinalee hi, any updates on this PR? |
Closing this in favour of #769 |
Fixes #580
Adds a lint for extra whitespace around type parameters which resolves the previously added failing tests.