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

[0.49.0] loadBaseline loads invalid absolute paths when invoked from separate process/with different working directory #1962

Closed
mateuszkwiecinski opened this issue Apr 21, 2023 · 5 comments · Fixed by #1967 or #1972
Milestone

Comments

@mateuszkwiecinski
Copy link
Contributor

Expected Behavior

I'd expect loadBaseline to expose relative paths, like 0.48.x did, or have a way to pass a root directory baseline entries should be resolved against (a loadBaseline(path = ..., workingDir = ...))?

Observed Behavior

Baseline#lintErrorsPerFile reference absolute paths pointing at temporary location created by Gradle, which is different than the one passed to KtlineRuleEngine#lint or #format.

Screenshot 2023-04-21 at 22 52 59
+ sample baseline entry:

    <file name="app/src/test/kotlin/io/github/usefulness/sample/OpSpacing.kt">
        <error line="5" column="16" source="standard:op-spacing" />
    </file>

(The screeshot lists result of loadBaseline(...).lintErrorsPerFile.keys and compares it with file paths passed to #lint or #format as Code.fromFile(file)

As far as I understand, loadBaseline creates absolute paths relatively to the place it was called. So when running in process isolation, when gradle try to run stuff safely, loadBaseline is effectively called from i.e. $GRADLE_HOME/workers which results in invalid absolute paths resolved.
0.48.0 returned relative paths, so plugins find absolute location on their own: example1, example2

Offending code: link + the comment that suggests different things than the function does, I guess 👀

Steps to Reproduce

  1. Generate non-empty baseline using ktlint --baseline=ktlint-baseline.xml
  2. Call loadBaseline() from a process with working directory outside of the directory the baseline was created from? or whatever Gradle Worker with process isolation does here 😬

Your Environment

  • Version of ktlint used: 0.49.0
  • Relevant parts of the .editorconfig settings - N/A
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): link
  • Version of Gradle used (if applicable): N/A
  • Operating System and version: N/A
@paul-dingemans paul-dingemans added this to the 0.49.1 milestone Apr 22, 2023
paul-dingemans added a commit that referenced this issue Apr 22, 2023
paul-dingemans added a commit that referenced this issue Apr 22, 2023
* Store path of file containing a lint violation relative to the path of the baseline file itself

Closes #1962
@paul-dingemans
Copy link
Collaborator

@mateuszkwiecinski Can you please verify whether the problem is solved for you in latest snapshot build?

@mateuszkwiecinski
Copy link
Contributor Author

mateuszkwiecinski commented Apr 23, 2023

@paul-dingemans Unfortunately I still observe the same behavior. I assume it's because the path passed to loadBaseline is i.e. ~/project/path/app/config/baseline.xml, so when a baseline entry points at src/main/kotlin/CustomClass.kt, the loadBaseline using

val fileName =
    Paths
        .get(getAttribute("name"))
        .absolutePathString()

loads ~/.gradle/workers/src/main/kotlin/CustomClass.kt, which means the newly added removePrefix is a no-op:
image

since ~/.gradle/workers/src/main/kotlin/CustomClass.kt does not start with ~/project/path/app/config/baseline.xml.

From my observation the absolutePathString resolves paths against the System.getProperty("user.dir") location 👀 But if your intention was to keep using relative paths, then removing absolutePathString should do the trick.

Or maybe I'm using the wrong code. I'm a bit of lost which artifacts are meant to be used by API consumers and which are only needed for CLI. Previously there was just one, now I have to depend on 4 of them

"com.pinterest:ktlint" // needed for `Baseline.kt` only
"com.pinterest.ktlint:ktlint-rule-engine" // core part of ktlint
"com.pinterest.ktlint:ktlint-cli-ruleset-core" // needed for `RuleSetProviderV3`, not sure how to resolve `RuleProvider`s using just `ktlint-rule-engine` 
"com.pinterest.ktlint:ktlint-cli-reporter" // no `-core`?

Baseline.kt I'm referring to comes from ktlint-cli module, so maybe I'm wrong by using it, as it contains CLI-specific implementation? Is there a Baseline.kt implementation for API consumers? 🤔 Is there a single artifact that all API consumers (including CLI) should use?

@paul-dingemans
Copy link
Collaborator

I have incorrectly assumed that the baseline file was always stored in the root of the project. Now, the baseline again will return the relative path to the file. But it needed a fix in the Ktlint CLI to retrieve the existing errors from the baseline so that errors in the baseline will not be reported.

The terminology "API Consumer" might be confusing. For me an API Consumer is a project that invokes the KtlintRuleEngine directly. Such API Consumers do not use the Ktlint CLI (and as of that non of the ktlint-cli modules).

Integrators that invoke the Ktlint CLI typically do not invoke the KtlintRuleEngine directly.

I am confused why you have a need for using the Baseline.kt from the Ktlint CLI but also need a direct dependency on ktlint-rule-engine. Do you invoke the Ktlint CLI or the lint/format functions of KtlintRuleEngine directly?

@paul-dingemans
Copy link
Collaborator

I missed the last part of your comment:

Baseline.kt I'm referring to comes from ktlint-cli module, so maybe I'm wrong by using it, as it contains CLI-specific implementation? Is there a Baseline.kt implementation for API consumers? 🤔 Is there a single artifact that all API consumers (including CLI) should use?

It looks like that you're (ab)using the Baseline.kt functionality from the Ktlint CLI. The KtlintRuleEngine has absolute no knowledge of the concept of a baseline. So I expect that a part of the baseline functionality of Ktlint CLI also has been 'copied/implementedin your project. If so, it would be best to copy theBaseline.kt` as well.

@paul-dingemans
Copy link
Collaborator

New snapshot is available with fix in Baseline.kt to return relative path.

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