Reuse supplied import options in ClassFileImporter.importClasspath() #328
Conversation
|
👉 View analysis in DeepCode’s Dashboard |
Before this commit the ImportOption returned true for all java.base classes _and_ for all classes that are not part of any module. From now on the ImportOption only returns true for java.base classes. Signed-off-by: Roland Weisleder <roland.weisleder@googlemail.com>
Before this commit, all custom import options were ignored and replaced with DO_NOT_INCLUDE_ARCHIVES. From this commit on DO_NOT_INCLUDE_ARCHIVES will be added to the already supplied import options. Fixes TNG#296 Signed-off-by: Roland Weisleder <roland.weisleder@googlemail.com>
I think unfortunately it was the name that was wrong, not the implementation. Originally the first test wanted to test that it would get as well JDK classes as library classes as project classes. The second part of the test is the opposite of the first part where it would assert that these classes are not present in the default (except for project). For the package test importing `java.base` is actually sufficient (and that commit was the one that gave it the unfortunate misleading name, simply reusing the import from the first test). I kept `importJavaBase` the way it is now (because the method name tells the truth) but inlined the old version in the first method where we want `java.base` classes as well as library and project classes. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
|
Thanks a lot! 😃 I adjusted the test name in your commit a little to better reflect the point of it (at least how I see it). Also I think the one test you adjusted actually should stay as it was, because the whole point was to get classes from the JDK, JARs and the project together. But unfortunately when I introduced the test for packages, I sloppily renamed that method to |
Somehow Spotbugs 3.x seems to have broken dependencies meanwhile, so I have upgraded it to the current version. Seems like the plugin API has changed meanwhile and the source sets cannot be manipulated anymore, thus I have adapted it to disable the test tasks (with all the many test examples we need for tests, Spotbugs reports a lot of useless warnings for tests, so we only enable it for production code). Also there now seems to be automatically one task per source set, so we do not need to manually add `sourceSets.api` anymore. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
|
Hmm, I think Travis CI is broken at the moment 😦 First I thought our Spotbugs version is too old (because it complained about some beta-version of a dependency missing?? 😮 Does not raise my trust into the dependency management of Spotbugs...) So I've upgraded Spotbugs to the current version, but now it fails about some other dependency supposedly missing in Maven Central. If I copy the URL that supposedly cannot be reached, I can access it, so I think there is something wrong in Travis CI. |
…328 Until now any `ImportOption` supplied via `classFileImporter.withImportOptions(importOptions)` was disregarded, if `importClasspath()` was used (that method just used new `ImportOptions` with some default). Now any previously added `ImportOptions` will be merged with the defaults resulting in a more consistent and less surprising development experience. Fixes #296
Until now any
ImportOptionsupplied viaclassFileImporter.withImportOptions(importOptions)was disregarded, ifimportClasspath()was used (that method just used newImportOptionswith some default).Now any previously added
ImportOptionswill be merged with the defaults resulting in a more consistent and less surprising development experience.Fixes #296