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

Fix logic "BinaryExpression" in "LineLength" Rule #1292

Merged
merged 38 commits into from
May 16, 2022
Merged

Fix logic "BinaryExpression" in "LineLength" Rule #1292

merged 38 commits into from
May 16, 2022

Conversation

Arrgentum
Copy link
Member

@Arrgentum Arrgentum commented May 4, 2022

Fix logic of the line split in BinaryExpression in rule LineLength

This pull request closes #1244

Actions checklist

  • Correct rule LineLength
  • Added and correct tests on fixers

@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #1292 (004e368) into master (6e60358) will decrease coverage by 0.09%.
The diff coverage is 87.38%.

@@             Coverage Diff              @@
##             master    #1292      +/-   ##
============================================
- Coverage     82.07%   81.97%   -0.10%     
- Complexity     2525     2527       +2     
============================================
  Files           105      105              
  Lines          7195     7169      -26     
  Branches       2057     2053       -4     
============================================
- Hits           5905     5877      -28     
- Misses          344      346       +2     
  Partials        946      946              
Flag Coverage Δ
unittests 81.97% <87.38%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...qfn/diktat/ruleset/rules/chapter1/PackageNaming.kt 89.83% <ø> (ø)
...g/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt 89.32% <87.38%> (-1.72%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e60358...004e368. Read the comment docs.

Arrgentum added 3 commits May 4, 2022 16:23
…_binary

# Conflicts:
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter1/PackageNaming.kt
@orchestr7
Copy link
Member

orchestr7 commented May 4, 2022

Alexey, please start naming PR properly :(

"Condition" - bad name ❌
"Condition for balboa to blabla" - good name

@Arrgentum Arrgentum changed the title Condition Fix Rule "LineLength" in "BinaryExpression" May 4, 2022
@Arrgentum Arrgentum changed the title Fix Rule "LineLength" in "BinaryExpression" Fix logic "BinaryExpression" in "LineLength" Rule May 4, 2022
Copy link
Member

@sanyavertolet sanyavertolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep an eye on how your branch is called! It should start with feature/, bugfix/, hotfix/ of infra/ (if I am not mistaken).

@Suppress("KDOC_NO_CONSTRUCTOR_PROPERTY", "MISSING_KDOC_CLASS_ELEMENTS") // todo add proper docs
sealed class LongLineFixableCases {
object None : LongLineFixableCases()
open class LongLineFixableCases(val node: ASTNode)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change sealed for open?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, I changed it back

/**
* Class None show error long line have unidentified type or something else that we can't analyze
*/
class None : LongLineFixableCases(KotlinParser().createNode("ERROR"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still can be an object

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do that, there will be something breaking in the logic

val indexLastSpace = wrongProperty.indexLastSpace
val text = wrongProperty.text
splitTextAndCreateNode(node, text, indexLastSpace)
@Suppress("TYPE_ALIAS", "UnsafeCallOnNullableType")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, that diktat doesn't trigger on this line, because there is no whitespaces, between suppress and previous method, probably this is because annotation type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@sanyavertolet sanyavertolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When more tasks with this rule are done, you will need to add tests that cover all the LongLine rule

@Arrgentum Arrgentum requested a review from petertrr May 16, 2022 11:29
@Arrgentum Arrgentum merged commit 4a13e39 into master May 16, 2022
@Arrgentum Arrgentum deleted the condition branch May 16, 2022 13:32
This was referenced Jun 21, 2022
@Arrgentum Arrgentum self-assigned this Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in rule LONG_LINE in condition
5 participants