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
Break the dependency of :detekt-api
with :detekt-psi-utils
#7140
Comments
Which class are we talking about? |
I'd love to get rid of that class. We have basePath, which should be required from tooling, or have a sensible default. If other paths are absolute, we can always generate the relative path from the absolute path and basePath. No need to store all versions of the path, nor do we need to convert anything to a FilePath instead of just using a Path. Relative paths are only needed when outputting results. Absolute paths should always be used internally. |
That's true! And it shouldn't be difficult to change. I'm going to give it a try. |
My findings: Right now on a
For sure we don't need 3 because with 2 of them we can calculate the other. So we could reduce it to:
But we can do it even better. Note: because we just have one path now on The problem with My proposal to avoid this is to change
What do you think? I'm going to implement it to see how it looks like (or at least try it). |
We should actually go further, but I don't think it's possible to solve while we support autocorrect in the way we do now. If we created each
Why is it available in compiler plugin, but not when we generate them? Because we use a different method to create each detekt/detekt-parser/src/main/kotlin/io/github/detekt/parser/KtCompiler.kt Lines 34 to 37 in 68b949f
While the Kotlin compiler will add the source files to the KotlinCoreEnvironment then use Why can't we do the same? Because
Since we now say autocorrect support is not available in detekt itself, but can be implemented in the rules or rule providers themselves, we could switch to using I personally think that's a reasonable tradeoff. We can easily get rid of |
Once my PRs are merged, it's trivial to create a PR to remove the last couple of detekt-psi-utils usages from detekt-api. However the question becomes, do we want to keep it as an Draft PR: #7260 |
No, we don't want to keep it as a dependency. It's better to declare it on every rule module. |
Agree on this 👍 I'd rather have explicit dependency on each ruleset rather than having |
Expected Behavior
:detekt-api
shouldn't depend on:detekt-psi-utils
.Current Behavior
It does
Context
This idea comes from this comment: #7105 (comment)
I did a fast check and the main reason to have this dependency is because
:detekt-api
usesFilePath
and that class is declared on:detekt-psi-utils
. That have little sense, that class should be defined on:detekt-api
. But once you move that class to:detekt-api
a lot of:detekt-psi-utils
doesn't compile because they useFilePath
. So maybe the solution is to invert the dependency, I'm not sure.Also this could help to unblock #7123.
The text was updated successfully, but these errors were encountered: