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

Function Signature flip-flops between violations with long expression body #1897

Closed
mklnwt opened this issue Mar 29, 2023 · 10 comments
Closed
Milestone

Comments

@mklnwt
Copy link

mklnwt commented Mar 29, 2023

Expected Behavior

function-signature does not flip-flop between violations.

Observed Behavior

function-signature with --format moves too long expression body to new line. Second run will move it back up.

It flip-flops between

"First line of body expression fits on same line as function signature"

and

"Newline expected before expression body"

Steps to Reproduce

function-signature with max_line_length of 120.

fun loremIpsumDolorSitAmetConsecteturAdipiscingElitMaecenasGravidaMaurisTortorAcUllamcorperDolorTristiqueEu(
    foo: String,
) = integerNecPortaQuamIntegerEuTemporNisiAcDignissimDolorSedRisusMagnaUltricesNonPorttitorUtCongueInMassaUtNisiTurpis()

fun integerNecPortaQuamIntegerEuTemporNisiAcDignissimDolorSedRisusMagnaUltricesNonPorttitorUtCongueInMassaUtNisiTurpis() { ... }

The first long method name is just to force breaking the parameter and haven the closing parenthesis in a new line.

The long method in the expression body can also be replaced with a long chained function call.

Your Environment

  • Version of ktlint used: 0.48.2
  • Operating System and version: Windows 11
@paul-dingemans
Copy link
Collaborator

I can not reproduce the problem wih the code sample and max_line_length of 120. But I expect this problem to be the same as #1808 which is already resolved in the upcoming 0.49.x release. If you want to, you can try the SNAPSHOT version (https://pinterest.github.io/ktlint/install/snapshot-build/) to verify it.

@mklnwt
Copy link
Author

mklnwt commented Apr 6, 2023

I was able to verify, thanks!

Sidenote: Versions after 0.49.0-20230214.082344-99.pom give an error.

ktlint-root:ktlint-ruleset-test-tooling:unspecified

because of

<dependency>
    <groupId>ktlint-root</groupId>
    <artifactId>ktlint-ruleset-test-tooling</artifactId>
    <version>unspecified</version>
    <scope>runtime</scope>
</dependency>

@paul-dingemans
Copy link
Collaborator

I was able to verify, thanks!

Sidenote: Versions after 0.49.0-20230214.082344-99.pom give an error.

ktlint-root:ktlint-ruleset-test-tooling:unspecified

because of

<dependency>
    <groupId>ktlint-root</groupId>
    <artifactId>ktlint-ruleset-test-tooling</artifactId>
    <version>unspecified</version>
    <scope>runtime</scope>
</dependency>

@Goooler Can you verify whether we need corrective action here?

According to command:

git log --full-history -- ktlint-ruleset-test-tooling/gradle.properties

I have added gradle.properties on Jan 29th when I restructured the modules. At Feb 15th (my time) this was merged. So it can match with the snapshot with version 0.49.0-20230214.082344-99.pom. I don't understand why its version and scope are unspecified and runtime. On March 25th, we decided to remove this artifact from BOM publishing (#1883) as the module is not to be published.

@Goooler
Copy link
Contributor

Goooler commented Apr 7, 2023

Some modules depend on ktlint-ruleset-test-tooling, but it hasn't been published to MavenCentral.

implementation(projects.ktlintRulesetTestTooling)

implementation(projects.ktlintRulesetTestTooling)

The reason why scope is runtime is you use implementation here, if you want compile in Maven need to api it in Gradle.

See:
https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_configurations_graph
https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#dependency-scope

@mklnwt
Copy link
Author

mklnwt commented Apr 7, 2023

@Goooler why is the version set for the other modules though?

Also why is ktlint-ruleset-test-tooling not on Sonatype snapshot repository.

<dependency>
    <groupId>com.pinterest.ktlint</groupId>
    <artifactId>ktlint-ruleset-standard</artifactId>
    <version>0.49.0-SNAPSHOT</version>
    <scope>runtime</scope>
</dependency>
<dependency>
    <groupId>ktlint-root</groupId>
    <artifactId>ktlint-ruleset-test-tooling</artifactId>
    <version>unspecified</version>
    <scope>runtime</scope>
</dependency>

Also is there a reason for using implementation(projects.ktlintRulesetTestTooling) instead of implementation(project(":ktlintRulesetTestTooling"))?

@Goooler
Copy link
Contributor

Goooler commented Apr 7, 2023

  1. Cause we don't publish ktlint-ruleset-test-tooling, just use it as a local dependency.
  2. projects.ktlintRulesetTestTooling equals to project(":ktlintRulesetTestTooling"), see Enable type-safe project accessors #1410.

@mklnwt
Copy link
Author

mklnwt commented Apr 7, 2023

Shouldn't it be compileOnly, testCompileOnly or testImplementation then?

api will still expose the library to consumers/users.

@paul-dingemans
Copy link
Collaborator

When I run the publishToMavenLocal task, the ktlint-ruleset-test-tooling is not written as artifact to my local maven repository as was expected. But the ktlint-bom which was produced indeed still contains the dependency below:

<dependency>
    <groupId>ktlint-root</groupId>
    <artifactId>ktlint-ruleset-test-tooling</artifactId>
    <version>unspecified</version>
    <scope>runtime</scope>
</dependency>

@paul-dingemans paul-dingemans added this to the 0.49.0 milestone Apr 7, 2023
@paul-dingemans
Copy link
Collaborator

Issue closed as original problem was a duplicate. The unspecified dependency has been resolved via #1925.

@paul-dingemans paul-dingemans closed this as not planned Won't fix, can't repro, duplicate, stale Apr 8, 2023
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

No branches or pull requests

3 participants