SLCORE-2242 Add embedded, unsupported and premium artifact resolvers#1924
Conversation
SummaryAdds four new artifact resolver implementations to handle different plugin/analyzer sources and availability states. These resolvers are part of the artifact resolution system that determines which language analyzers are available (embedded, premium, unsupported, or downloadable). The changes include comprehensive test coverage for each resolver. What reviewers should knowStart with the resolver implementations in
The corresponding tests in Key architectural notes: These resolvers work with
|
There was a problem hiding this comment.
Conclusion: The resolver logic is clean and the tests are thorough, but there are two correctness concerns worth addressing before merge — the resolveCompanionPlugins parameter that is silently ignored (ambiguous whether by design or an oversight), and a subtle behavioural dependency between UnsupportedArtifactResolver and PremiumArtifactResolver that is invisible at each call site.
| .collect(Collectors.toMap( | ||
| Map.Entry::getKey, | ||
| e -> PluginStatus.forCompanion(e.getKey(), ArtifactState.ACTIVE, ArtifactSource.EMBEDDED, e.getValue()))); | ||
| } |
There was a problem hiding this comment.
resolveCompanionPlugins accepts connectionId but unconditionally iterates standaloneEmbeddedPathsByKey, ignoring the parameter entirely. Companion plugins are always taken from the standalone embedded set regardless of mode.
If intentional (companion plugins are always the same across modes), the parameter should be removed from the interface or at least documented. If they can differ per connection, this is a bug. The test suite only exercises this with null, leaving connected-mode behaviour unverified either way.
- Mark as noise
There was a problem hiding this comment.
Pull request overview
This PR introduces a set of artifact resolvers in backend/core to classify/locate analyzers (embedded plugins, embedded extra artifacts like Omnisharp distributions, and “premium/unsupported” states) and adds unit tests covering the new resolvers’ behaviors.
Changes:
- Add
EmbeddedArtifactResolverto resolve embedded plugin JARs (standalone + connected mode) and expose companion plugins. - Add
EmbeddedExtraArtifactResolverto resolve embedded “extra” artifacts (Omnisharp distribution paths) fromInitializeParams. - Add
PremiumArtifactResolverandUnsupportedArtifactResolverplus corresponding unit tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/core/src/main/java/org/sonarsource/sonarlint/core/plugin/resolvers/UnsupportedArtifactResolver.java | Adds resolver to return an UNSUPPORTED artifact when a language isn’t enabled. |
| backend/core/src/main/java/org/sonarsource/sonarlint/core/plugin/resolvers/PremiumArtifactResolver.java | Adds resolver to return a PREMIUM artifact for connected-only languages. |
| backend/core/src/main/java/org/sonarsource/sonarlint/core/plugin/resolvers/EmbeddedExtraArtifactResolver.java | Adds resolver for embedded extra artifacts (Omnisharp distro paths) keyed by artifact key. |
| backend/core/src/main/java/org/sonarsource/sonarlint/core/plugin/resolvers/EmbeddedArtifactResolver.java | Adds embedded plugin resolution (standalone/connected) and companion plugin detection. |
| backend/core/src/test/java/org/sonarsource/sonarlint/core/plugin/UnsupportedArtifactResolverTest.java | Adds tests for unsupported resolution behavior. |
| backend/core/src/test/java/org/sonarsource/sonarlint/core/plugin/PremiumArtifactResolverTest.java | Adds tests for premium resolution behavior. |
| backend/core/src/test/java/org/sonarsource/sonarlint/core/plugin/EmbeddedExtraArtifactResolverTest.java | Adds tests for extra-artifact path resolution. |
| backend/core/src/test/java/org/sonarsource/sonarlint/core/plugin/EmbeddedArtifactResolverTest.java | Adds tests for embedded plugin and companion plugin resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // --- Disabled-keys behaviour --- | ||
|
|
||
| @Test | ||
| void should_return_empty_when_plugin_key_is_disabled_for_analysis_in_standalone() { | ||
| var languageSupport = mockLanguageSupport(true, true); | ||
| var resolver = new UnsupportedArtifactResolver(languageSupport); | ||
|
|
||
| var result = resolver.resolve(SonarLanguage.JAVA, null); | ||
|
|
||
| assertThat(result).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void should_return_empty_when_plugin_key_is_disabled_for_analysis_in_connected_mode() { | ||
| var languageSupport = mockLanguageSupport(true, true); | ||
| var resolver = new UnsupportedArtifactResolver(languageSupport); |
|
|
||
| private static Map<String, Path> buildPluginKeyToPathMap(Set<Path> embeddedPaths) { | ||
| return embeddedPaths.stream() | ||
| .collect(Collectors.toMap(p -> SonarPluginManifest.fromJar(p).getKey(), Function.identity())); |
| @Test | ||
| void should_resolve_to_active_embedded_in_standalone_when_filename_contains_plugin_key() throws IOException { | ||
| var javaJar = createJar("sonar-java-plugin.jar"); | ||
| var resolver = new EmbeddedArtifactResolver(mockParams(Set.of(javaJar), Map.of(), null)); | ||
| var expected = resolved(ArtifactState.ACTIVE, javaJar, ArtifactSource.EMBEDDED); | ||
|
|
||
| var result = resolver.resolve(SonarLanguage.JAVA, null); | ||
|
|
||
| assertThat(result).contains(expected); | ||
| } |
| if (languageSupportRepository.isEnabledInConnectedMode(language) | ||
| && !languageSupportRepository.isEnabledInStandaloneMode(language)) { |
| return (languageSupportRepository.isEnabledInStandaloneMode(language) && connectionId == null) | ||
| || languageSupportRepository.isEnabledInConnectedMode(language); |
| connected.forEach(lang -> when(repo.isEnabledInConnectedMode(lang)).thenReturn(true)); | ||
| standalone.forEach(lang -> when(repo.isEnabledInStandaloneMode(lang)).thenReturn(true)); |
| private static LanguageSupportRepository mockLanguageSupport(boolean enabledInStandalone, boolean enabledInConnected) { | ||
| var repo = mock(LanguageSupportRepository.class); | ||
| when(repo.isEnabledInStandaloneMode(SonarLanguage.JAVA)).thenReturn(enabledInStandalone); | ||
| when(repo.isEnabledInConnectedMode(SonarLanguage.JAVA)).thenReturn(enabledInConnected); | ||
| when(repo.isEnabledInStandaloneMode(SonarLanguage.PYTHON)).thenReturn(enabledInStandalone); | ||
| when(repo.isEnabledInConnectedMode(SonarLanguage.PYTHON)).thenReturn(enabledInConnected); | ||
| return repo; |
94327f2 to
aabe95d
Compare
No description provided.