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

can't modify unresolved.dependencies #704

Closed
jonathanlermitage opened this issue Oct 18, 2022 · 6 comments · Fixed by #708
Closed

can't modify unresolved.dependencies #704

jonathanlermitage opened this issue Oct 18, 2022 · 6 comments · Fixed by #708

Comments

@jonathanlermitage
Copy link
Contributor

jonathanlermitage commented Oct 18, 2022

Starting from version 0.43.0 I can no longer modify the value of the unresolved.dependencies property.
Example: https://github.com/jonathanlermitage/intellij-extra-icons-plugin/blob/master/build.gradle.kts#L178-L187

withType<DependencyUpdatesTask> {
        outputFormatter = closureOf<Result> {
            unresolved.dependencies.removeIf {
                val coordinates = "${it.group}:${it.name}"
                coordinates.startsWith("unzipped.com") || coordinates.startsWith("com.jetbrains:ideaI")
            }
            val plainTextReporter = PlainTextReporter(project, revision, gradleReleaseChannel)
            val writer = StringWriter()
            plainTextReporter.write(writer, this)
            logger.quiet(writer.toString().trim())
        }
}

(I'm doing this because Gradle can't find some artifacts version, like com.jetbrains:ideaIU. This is not a blocker issue, this is purely cosmetic. I'm curious... ^_^)

Actually, Gradle's error is weird: I ran gradlew dependencyUpdates and I got this error:

./gradlew dependencyUpdates --warning-mode all

> Configure project :
e: /home/jon/prj/intellij-extra-icons-plugin/build.gradle.kts:179:37: Unresolved reference: removeIf
e: /home/jon/prj/intellij-extra-icons-plugin/build.gradle.kts:180:38: Unresolved reference: it
e: /home/jon/prj/intellij-extra-icons-plugin/build.gradle.kts:180:50: Unresolved reference: it
e: /home/jon/prj/intellij-extra-icons-plugin/build.gradle.kts:185:37: Type mismatch: inferred type is StringWriter but OutputStream was expected

FAILURE: Build failed with an exception.

* Where:
Build file '/home/jon/prj/intellij-extra-icons-plugin/build.gradle.kts' line: 179

* What went wrong:
Script compilation errors:

  Line 179:             unresolved.dependencies.removeIf {
                                                ^ Unresolved reference: removeIf

  Line 180:                 val coordinates = "${it.group}:${it.name}"
                                                 ^ Unresolved reference: it

  Line 180:                 val coordinates = "${it.group}:${it.name}"
                                                             ^ Unresolved reference: it

  Line 185:             plainTextReporter.write(writer, this)
                                                ^ Type mismatch: inferred type is StringWriter but OutputStream was expected

4 errors

Line 179 refers to unresolved.dependencies.removeIf.
It makes no sens. This is a SortedSet (I checked this with a debugger), so I should be able to invoke removeIf. I also tried to invoke clear() and I faced the same issue. But it seems to be a Set, because I can iterate over this set with foreach.

Maybe this is a side effect of the recent Kotlin migration? Some Kotlin basic collections are immutable. The debugger says this is a SortedSet from java.util, but I already observed some bugs with JDK17 and Kotlin: some lists from java.util were silently replaced by their equivalent class from Kotlin standard lib; at least for methods which declares a List return type but actually returns a java.util.ArrayList. It made me crazy. I finally fixed this by replacing the java.util.list return type by java.util.ArrayList, and replacing star imports of java.util by single-class imports, otherwise Kotlin seemed to pick the ArrayList from the Kotlin standard lib instead of java.util.*. It was super weird because it did not fail at compile-time, but at runtime, with Java17 only (no problem with Java11): ArrayList from Kotlin std lib couldn't be casted to java.util.ArrayList ^_^.

@jonathanlermitage
Copy link
Contributor Author

I also tried to copy unresolved.dependencies to a new SortedSet (lets name it mySet), remove unwanted items from this set, then do unresolved.dependencies = mySet, but the compiler says it cannot modify unresolved.dependencies value. It seems to be a val, not a var.

@ben-manes
Copy link
Owner

Thanks @jonathanlermitage. I'm not very familiar with Kotlin yet, but you are welcome to send a PR with your improvement ideas.

@jonathanlermitage
Copy link
Contributor Author

Sure, no problem, I will look at this.

@jonathanlermitage
Copy link
Contributor Author

@ben-manes I was able to update unresolved.dependencies by changing its type from Set to TreeSet, but I'm not happy with this workaround. We should keep unmodifiable collections, this is a good thing.
Instead, I added a parameter called ignoreUnresolvedDependenciesPattern (a regex) which can be used to filter the unresolved dependencies list. The gradle script looks like this:

withType<DependencyUpdatesTask> {
    ...
    ignoreUnresolvedDependenciesPattern = "com\\.jetbrains.*|unzipped\\..*"
    ...
}

I will see if I can add some unit test, then I will create a pull request very soon.
Before I do that, I would like to know your opinion on this. Are you OK with this solution?
Also, I am very bad at naming things 😁 Do you think ignoreUnresolvedDependenciesPattern is a good name for this feature?
Thx!

@ben-manes
Copy link
Owner

ben-manes commented Nov 11, 2022

I think you might use a MutableSet instead?

I agree immutability is a nice practice, but mutability is simple for any manipulations like your original example. If one has other use cases then the pattern has to be extended to more categories. So I slightly prefer mutability for scripting as it helps get the job done and is not in a usage that can cause trouble. Otherwise if there is just as easy way via builders then you can get equivalent behaviors. wdyt?

@jonathanlermitage
Copy link
Contributor Author

I see, good idea! I will create a PR asap with a MutableSet 😀

jonathanlermitage added a commit to jonathanlermitage/gradle-versions-plugin that referenced this issue Nov 12, 2022
jonathanlermitage added a commit to jonathanlermitage/gradle-versions-plugin that referenced this issue Nov 12, 2022
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

Successfully merging a pull request may close this issue.

2 participants