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

MaxLineLength should not raise any finding inside a raw string #5079

Closed
wants to merge 1 commit into from

Conversation

gouri-panda
Copy link
Contributor

closes: #5060

@codecov
Copy link

codecov bot commented Jul 15, 2022

Codecov Report

Merging #5079 (0c7ac53) into main (6bce28f) will increase coverage by 0.00%.
The diff coverage is 76.92%.

@@            Coverage Diff            @@
##               main    #5079   +/-   ##
=========================================
  Coverage     84.87%   84.87%           
- Complexity     3575     3576    +1     
=========================================
  Files           500      500           
  Lines         11785    11793    +8     
  Branches       2197     2198    +1     
=========================================
+ Hits          10002    10009    +7     
  Misses          692      692           
- Partials       1091     1092    +1     
Impacted Files Coverage Δ
...lab/arturbosch/detekt/rules/style/MaxLineLength.kt 93.75% <76.92%> (-1.25%) ⬇️

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 6bce28f...0c7ac53. Read the comment docs.

Copy link
Contributor

@marschwar marschwar left a comment

Choose a reason for hiding this comment

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

I am sorry, but this does not work at all. Your test cases seem to "work" because there is a return statement that prevents further checks. Also there are some test cases that I assume should be failing if we allow multi line strings to contain long lines.

@gouri-panda
Copy link
Contributor Author

@marschwar I'll add some more different tests 👍

@@ -97,6 +108,7 @@ class MaxLineLength(config: Config = Config.empty) : Rule(config) {
companion object {
private const val DEFAULT_IDEA_LINE_LENGTH = 120
private val BLANK_OR_QUOTES = """[\s"]*""".toRegex()
private const val TQ = "\"\"\""
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't need to look for """. You should check the parent of those chars that are "MaxLineLength" and see if its type is KtStringTemplateExpression.

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

Successfully merging this pull request may close these issues.

MaxLineLength and raw strings
3 participants