Skip to content

Cleaner merging of Gradle blocks for functionalTests #5830

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

Merged
merged 3 commits into from
Feb 26, 2023

Conversation

TWiStErRob
Copy link
Member

@TWiStErRob TWiStErRob commented Feb 24, 2023

Follow-up on #5819 (comment)
Related #5827
Closes #5828

Changes:

  • Fix indentation of fixture code (somewhere 4, somewhere 3, somewhere 2 spaces), yaml kept at 2 everything else is 4.
  • Replace all remaining trimMargin with trimIndent
  • Add some @Language annotations, the build code in functionalTests will be highlighted properly (requires trimMargin removal).
  • Re-think how multiple pieces of Gradle script blocks are merged.
    • Previously
      • they were relying on the | being present in the input string,
      • some of them didn't have the leading margin marker.
      • simple string concatenation was done with triple-quotes
      • the generated output was still messy, mis-indented, and hard to follow.
    • In this PR
      • Removed all @Suppress("TrimMultilineRawString")
      • Each code block is self-contained
      • This is achieved by 2 simple rules:
        1. Apply trimIndent() at the definition of the fragment
        2. Deal with indentation where the merging happens
      • This needed two small functions to be introduced: reIndent and mergeGradleBlocks.
        Their usages are pretty localized and make the fragment re-use be at a higher level of abstraction (like """$a $b""" -> merge(a, b)).

You can look at the resulting changes by diffing these two files (note: console output trims empty lines so the effect of \n\n is not clearly visible here, but it is in the JUnit test report):
main.txt
pr.txt

@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #5830 (36079c1) into main (059afea) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #5830   +/-   ##
=========================================
  Coverage     84.59%   84.59%           
  Complexity     3790     3790           
=========================================
  Files           546      546           
  Lines         12918    12918           
  Branches       2268     2268           
=========================================
  Hits          10928    10928           
  Misses          861      861           
  Partials       1129     1129           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@TWiStErRob TWiStErRob added the housekeeping Marker for housekeeping tasks and refactorings label Feb 24, 2023
@TWiStErRob TWiStErRob added this to the 1.23.0 milestone Feb 24, 2023
@@ -25,37 +27,40 @@ constructor(
private val rootDir: File = Files.createTempDirectory("applyPlugin").toFile().apply { deleteOnExit() }
private val randomString = UUID.randomUUID().toString()

@Language("gradle.kts")
Copy link
Member

Choose a reason for hiding this comment

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

TIL you can do this with "gradle.kts" 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

Funny thing: I only learned this 2 days ago while writing this PR too :)

Before that I knew kotlin and gradle were languages, but recently I was having chats about .gradle.kts files as convention plugins (which are way awesome btw), so while I was writing these annotations I just naturally wrote it, and it worked! 😅

Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

👏👏👏 Good one! And the diff of those two files looks WAY WAY better.

@BraisGabin BraisGabin merged commit e9ed339 into detekt:main Feb 26, 2023
@TWiStErRob TWiStErRob deleted the trimMargin-func branch February 26, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gradle-plugin housekeeping Marker for housekeeping tasks and refactorings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants