-
Notifications
You must be signed in to change notification settings - Fork 725
SONARJAVA-6490 Implement S8910: Interfaces with @Mapper should have @DaoFactory methods #5690
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
Open
romainbrenguier
wants to merge
17
commits into
master
Choose a base branch
from
new-rule/SONARJAVA-6490-S8910
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+499
−2
Open
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
af9fc21
Implement S8910 inherited DaoFactory detection
romainbrenguier 934ba15
Improve test coverage for S8910 MapperWithoutDaoFactoryCheck
romainbrenguier 701c66f
Remove undefined UnknownType reference from test sample
romainbrenguier 130971a
Handle unresolved supertypes gracefully in S8910
romainbrenguier 13bbacd
Handle unresolved supertypes gracefully in S8910
romainbrenguier 3285726
Improve test coverage for S8910 MapperWithoutDaoFactoryCheck
romainbrenguier 16e8cf1
Add diamond inheritance test case for S8910
romainbrenguier acfa855
Add non-compliant diamond inheritance test for S8910
romainbrenguier 15c3dcb
Merge remote-tracking branch 'origin/master' into new-rule/SONARJAVA-…
romainbrenguier d7f27e5
Merge remote-tracking branch 'origin/master' into new-rule/SONARJAVA-…
romainbrenguier fbb7b73
Address review comment from gitar-bot on java-checks/src/main/java/or…
romainbrenguier 3b97b7d
Address review comment from NoemieBenard on java-checks-test-sources/…
romainbrenguier 6a13976
Address review comment from NoemieBenard on java-checks-test-sources/…
romainbrenguier 19a20a4
Fix CI failures
romainbrenguier 454eb4e
Fix CI: Fixed invalid Noncompliant comment format in MapperWithoutDao…
romainbrenguier e8f7818
Merge remote-tracking branch 'origin/master' into new-rule/SONARJAVA-…
romainbrenguier ed31db3
Fix CI: Fixed incorrect expected rule count in JavaAgenticWayProfileT…
romainbrenguier File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
6 changes: 6 additions & 0 deletions
6
its/autoscan/src/test/resources/autoscan/diffs/diff_S8910.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "ruleKey": "S8910", | ||
| "hasTruePositives": true, | ||
| "falseNegatives": 0, | ||
| "falsePositives": 0 | ||
| } |
20 changes: 20 additions & 0 deletions
20
...urces/default/src/main/files/non-compiling/checks/MapperWithoutDaoFactoryCheckSample.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package checks; | ||
|
|
||
| import com.datastax.oss.quarkus.runtime.api.mapper.Mapper; | ||
|
|
||
| class MapperWithoutDaoFactoryCheckSample { | ||
|
|
||
| // Case with unresolved supertype - should not raise issue to avoid false positive | ||
| @Mapper | ||
| interface MapperWithUnresolvedSupertype extends UnresolvedInterface { | ||
| } | ||
|
|
||
| // Case with partially resolved inheritance | ||
| @Mapper | ||
| interface MapperExtendingResolvableAndUnresolvable extends ResolvableInterface, AnotherUnresolvedInterface { | ||
| } | ||
|
|
||
| interface ResolvableInterface { | ||
| String getName(); | ||
| } | ||
| } |
206 changes: 206 additions & 0 deletions
206
...-checks-test-sources/default/src/main/java/checks/MapperWithoutDaoFactoryCheckSample.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,206 @@ | ||
| package checks; | ||
|
|
||
| import com.datastax.oss.quarkus.runtime.api.mapper.Mapper; | ||
| import com.datastax.oss.quarkus.runtime.api.mapper.DaoFactory; | ||
|
|
||
| class MapperWithoutDaoFactoryCheckSample { | ||
|
|
||
| @Mapper | ||
| interface EmptyMapper { // Noncompliant {{Add at least one "@DaoFactory" method to this "@Mapper" interface.}} [[sc=13;ec=24]] | ||
| } | ||
|
|
||
| @Mapper | ||
| public interface FruitMapper { // Noncompliant | ||
| } | ||
|
|
||
| @Mapper | ||
| interface MapperWithOtherMethods { // Noncompliant | ||
| String getVersion(); | ||
| default int count() { | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| @Mapper | ||
| interface ExtendingMapper extends BaseInterface { | ||
| } | ||
|
|
||
| interface BaseInterface { | ||
| @DaoFactory | ||
| BaseDao baseDao(); | ||
| } | ||
|
|
||
| interface BaseDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| public interface CompliantFruitMapper { | ||
| @DaoFactory | ||
| FruitDao fruitDao(); | ||
| } | ||
|
|
||
| interface FruitDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface MultipleFactories { | ||
| @DaoFactory | ||
| UserDao userDao(); | ||
|
|
||
| @DaoFactory | ||
| ProductDao productDao(); | ||
| } | ||
|
|
||
| interface UserDao { | ||
| } | ||
|
|
||
| interface ProductDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface FactoryWithParameter { | ||
| @DaoFactory | ||
| KeyspaceDao dao(String keyspace); | ||
| } | ||
|
|
||
| interface KeyspaceDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface MixedMethods { | ||
| @DaoFactory | ||
| OrderDao orderDao(); | ||
|
|
||
| String getVersion(); | ||
| } | ||
|
|
||
| interface OrderDao { | ||
| } | ||
|
|
||
| // Compliant: the rule only checks @Mapper interfaces. | ||
| @Mapper | ||
| abstract class MapperClass { | ||
| } | ||
|
|
||
| // Compliant: the rule only checks @Mapper interfaces. | ||
| @Mapper | ||
| enum MapperEnum { | ||
| } | ||
|
|
||
| // Compliant: the rule only checks @Mapper interfaces. | ||
| @Mapper | ||
| record MapperRecord() { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface MultiLevelInheritance extends IntermediateInterface { | ||
| } | ||
|
|
||
| interface IntermediateInterface extends BaseFactoryInterface { | ||
| } | ||
|
|
||
| interface BaseFactoryInterface { | ||
| @DaoFactory | ||
| MultiLevelDao dao(); | ||
| } | ||
|
|
||
| interface MultiLevelDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface MultipleInheritance extends Interface1, Interface2 { | ||
| } | ||
|
|
||
| interface Interface1 { | ||
| String method1(); | ||
| } | ||
|
|
||
| interface Interface2 { | ||
| @DaoFactory | ||
| MultiInheritDao dao(); | ||
| } | ||
|
|
||
| interface MultiInheritDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface ComplexInheritance extends InterfaceWithFactory, InterfaceWithoutFactory { | ||
| } | ||
|
|
||
| interface InterfaceWithFactory { | ||
| @DaoFactory | ||
| ComplexDao dao(); | ||
| } | ||
|
|
||
| interface InterfaceWithoutFactory { | ||
| String getName(); | ||
| } | ||
|
|
||
| interface ComplexDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface CircularReference extends SelfReferencingBase { | ||
| } | ||
|
|
||
| interface SelfReferencingBase { | ||
| @DaoFactory | ||
| CircularDao dao(); | ||
| } | ||
|
|
||
| interface CircularDao { | ||
| } | ||
|
|
||
| @Mapper | ||
| interface DeepInheritanceChain extends Level1 { | ||
| } | ||
|
|
||
| interface Level1 extends Level2 { | ||
| } | ||
|
|
||
| interface Level2 extends Level3 { | ||
| } | ||
|
|
||
| interface Level3 { | ||
| @DaoFactory | ||
| DeepDao dao(); | ||
| } | ||
|
|
||
| interface DeepDao { | ||
| } | ||
|
|
||
| // Diamond inheritance pattern - tests visiting same interface multiple times | ||
| @Mapper | ||
| interface DiamondMapper extends DiamondLeft, DiamondRight { | ||
| } | ||
|
|
||
| interface DiamondLeft extends DiamondBase { | ||
| } | ||
|
|
||
| interface DiamondRight extends DiamondBase { | ||
| } | ||
|
|
||
| interface DiamondBase { | ||
| @DaoFactory | ||
| DiamondDao dao(); | ||
| } | ||
|
|
||
| interface DiamondDao { | ||
| } | ||
|
|
||
| // Diamond without DaoFactory - tests visited set when no factory found | ||
| @Mapper | ||
| interface DiamondWithoutFactory extends DiamondLeftNoFactory, DiamondRightNoFactory { // Noncompliant | ||
| } | ||
|
|
||
| interface DiamondLeftNoFactory extends DiamondBaseNoFactory { | ||
| } | ||
|
|
||
| interface DiamondRightNoFactory extends DiamondBaseNoFactory { | ||
| } | ||
|
|
||
| interface DiamondBaseNoFactory { | ||
| String getData(); | ||
| } | ||
|
|
||
| } |
27 changes: 27 additions & 0 deletions
27
...sources/default/src/main/java/com/datastax/oss/quarkus/runtime/api/mapper/DaoFactory.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| /* | ||
| * SonarQube Java | ||
| * Copyright (C) SonarSource Sàrl | ||
| * mailto:info AT sonarsource DOT com | ||
| * | ||
| * You can redistribute and/or modify this program under the terms of | ||
| * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the Sonar Source-Available License for more details. | ||
| * | ||
| * You should have received a copy of the Sonar Source-Available License | ||
| * along with this program; if not, see https://sonarsource.com/license/ssal/ | ||
| */ | ||
| package com.datastax.oss.quarkus.runtime.api.mapper; | ||
|
|
||
| import java.lang.annotation.ElementType; | ||
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.RetentionPolicy; | ||
| import java.lang.annotation.Target; | ||
|
|
||
| @Target(ElementType.METHOD) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| public @interface DaoFactory { | ||
| } |
27 changes: 27 additions & 0 deletions
27
...est-sources/default/src/main/java/com/datastax/oss/quarkus/runtime/api/mapper/Mapper.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| /* | ||
| * SonarQube Java | ||
| * Copyright (C) SonarSource Sàrl | ||
| * mailto:info AT sonarsource DOT com | ||
| * | ||
| * You can redistribute and/or modify this program under the terms of | ||
| * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the Sonar Source-Available License for more details. | ||
| * | ||
| * You should have received a copy of the Sonar Source-Available License | ||
| * along with this program; if not, see https://sonarsource.com/license/ssal/ | ||
| */ | ||
| package com.datastax.oss.quarkus.runtime.api.mapper; | ||
|
|
||
| import java.lang.annotation.ElementType; | ||
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.RetentionPolicy; | ||
| import java.lang.annotation.Target; | ||
|
|
||
| @Target(ElementType.TYPE) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| public @interface Mapper { | ||
| } |
95 changes: 95 additions & 0 deletions
95
java-checks/src/main/java/org/sonar/java/checks/MapperWithoutDaoFactoryCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| /* | ||
| * SonarQube Java | ||
| * Copyright (C) SonarSource Sàrl | ||
| * mailto:info AT sonarsource DOT com | ||
| * | ||
| * You can redistribute and/or modify this program under the terms of | ||
| * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the Sonar Source-Available License for more details. | ||
| * | ||
| * You should have received a copy of the Sonar Source-Available License | ||
| * along with this program; if not, see https://sonarsource.com/license/ssal/ | ||
| */ | ||
| package org.sonar.java.checks; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import org.sonar.check.Rule; | ||
| import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; | ||
| import org.sonar.plugins.java.api.semantic.Symbol; | ||
| import org.sonar.plugins.java.api.semantic.Type; | ||
| import org.sonar.plugins.java.api.tree.AnnotationTree; | ||
| import org.sonar.plugins.java.api.tree.ClassTree; | ||
| import org.sonar.plugins.java.api.tree.MethodTree; | ||
| import org.sonar.plugins.java.api.tree.Tree; | ||
|
|
||
| @Rule(key = "S8910") | ||
| public class MapperWithoutDaoFactoryCheck extends IssuableSubscriptionVisitor { | ||
|
|
||
| private static final String MAPPER_ANNOTATION = "com.datastax.oss.quarkus.runtime.api.mapper.Mapper"; | ||
| private static final String DAO_FACTORY_ANNOTATION = "com.datastax.oss.quarkus.runtime.api.mapper.DaoFactory"; | ||
| private static final String MESSAGE = "Add at least one \"@DaoFactory\" method to this \"@Mapper\" interface."; | ||
|
|
||
| @Override | ||
| public List<Tree.Kind> nodesToVisit() { | ||
| return Collections.singletonList(Tree.Kind.INTERFACE); | ||
| } | ||
|
|
||
| @Override | ||
| public void visitNode(Tree tree) { | ||
| ClassTree classTree = (ClassTree) tree; | ||
|
|
||
| if (!hasAnnotation(classTree.modifiers().annotations(), MAPPER_ANNOTATION)) { | ||
| return; | ||
| } | ||
|
|
||
| boolean hasExplicitSuperInterfaces = !classTree.superInterfaces().isEmpty(); | ||
| if (!hasDaoFactoryMethod(classTree.symbol(), hasExplicitSuperInterfaces)) { | ||
| reportIssue(classTree.simpleName(), MESSAGE); | ||
| } | ||
| } | ||
|
|
||
| private static boolean hasDaoFactoryMethod(Symbol.TypeSymbol typeSymbol, boolean hasExplicitSuperInterfaces) { | ||
| // Check the mapper's own members | ||
| if (typeSymbol.memberSymbols().stream() | ||
| .filter(Symbol::isMethodSymbol) | ||
| .map(Symbol.MethodSymbol.class::cast) | ||
| .anyMatch(MapperWithoutDaoFactoryCheck::hasDaoFactoryAnnotation)) { | ||
|
gitar-bot[bot] marked this conversation as resolved.
|
||
| return true; | ||
| } | ||
|
|
||
| // Check all supertypes (superTypes() already returns the full transitive closure) | ||
| for (Type superType : typeSymbol.superTypes()) { | ||
| Symbol.TypeSymbol superTypeSymbol = superType.symbol(); | ||
| if (superTypeSymbol.isUnknown() && hasExplicitSuperInterfaces) { | ||
| // If the interface explicitly extends other types and we encounter an unresolved supertype, | ||
| // assume it might provide the required @DaoFactory method to avoid false positives | ||
| // in projects with incomplete classpaths | ||
| return true; | ||
| } | ||
| if (!superTypeSymbol.isUnknown() && superTypeSymbol.memberSymbols().stream() | ||
| .filter(Symbol::isMethodSymbol) | ||
| .map(Symbol.MethodSymbol.class::cast) | ||
| .anyMatch(MapperWithoutDaoFactoryCheck::hasDaoFactoryAnnotation)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| private static boolean hasDaoFactoryAnnotation(Symbol.MethodSymbol methodSymbol) { | ||
| MethodTree declaration = methodSymbol.declaration(); | ||
| return declaration != null | ||
| ? hasAnnotation(declaration.modifiers().annotations(), DAO_FACTORY_ANNOTATION) | ||
| : methodSymbol.metadata().isAnnotatedWith(DAO_FACTORY_ANNOTATION); | ||
| } | ||
|
|
||
| private static boolean hasAnnotation(List<AnnotationTree> annotations, String fullyQualifiedName) { | ||
| return annotations.stream().anyMatch(annotation -> annotation.annotationType().symbolType().is(fullyQualifiedName)); | ||
| } | ||
|
|
||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.