Skip to content

SONARJAVA-6825: Implemented rule S9360 - Javadoc references should point to existing types - #6005

Closed
romainbrenguier wants to merge 13 commits into
masterfrom
romain/new-rule-s9360-sonarjava-6825
Closed

SONARJAVA-6825: Implemented rule S9360 - Javadoc references should point to existing types#6005
romainbrenguier wants to merge 13 commits into
masterfrom
romain/new-rule-s9360-sonarjava-6825

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Implements rule S9360: Javadoc @see and {@link}/{@linkplain} references should point to existing types
  • Uses semantic analysis via Sema.getClassType() to verify type references

Key features

  • Validates both @see block tags and {@link}/{@linkplain} inline tags
  • Resolves simple names via imports, java.lang.*, current package, and locally declared types
  • Handles inner class references with dot-to-$ notation (e.g., Outer.Inner -> Outer$Inner)
  • Handles multi-level nested inner classes (e.g., Outer.Inner.Deep -> Outer$Inner$Deep)
  • Skips external URLs, HTML anchors, quoted strings, and member-only references (#method)
  • No issues raised without semantic analysis

Test plan

  • Unit tests pass (14 tests)
  • Ruling baselines updated for S9360
  • Ruling QA CI checks pass
  • Plugin QA CI checks pass

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6825

@datadog-sonarsource

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #6006

Please review and merge it into your branch.

…aselines

- Fix dead code in resolveReference(): remove tautological guard that
  prevented relative (same-package) reference resolution
- Fix package name extraction: use ExpressionsHelper.concatenate()
  instead of unreliable PackageDeclarationTree.toString()
- Fix SEE_ANCHOR_PATTERN: match {@link}/{@linkplain} instead of
  invalid {@see} inline syntax
- Refactor visitTrivia loop to reduce break/continue (S135 fix)
- Add unit tests for extractSeeReferences and resolveReference methods
- Add test cases for {@link}, {@linkplain}, simple name references
- Add LITS ruling baselines for S9360

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 6 rule files: 0 issues removed, 414 issues added.

S9360 (java) on commons-beanutils - 0 issues removed, 3 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 251)

       246 |      * @throws NoSuchMethodException if there is no such accessible method
       247 |      * @throws InvocationTargetException wraps an exception thrown by the
       248 |      *  method invoked
       249 |      * @throws IllegalAccessException if the requested method is not accessible
       250 |      *  via reflection
>>>    251 |      */
       252 |     public static Object invokeMethod(
       253 |             final Object object,
       254 |             final String methodName,
       255 |             Object[] args,
       256 |             Class<?>[] parameterTypes)

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 574)

       569 |      * @throws InvocationTargetException wraps an exception thrown by the
       570 |      *  method invoked
       571 |      * @throws IllegalAccessException if the requested method is not accessible
       572 |      *  via reflection
       573 |      * @since 1.8.0
>>>    574 |      */
       575 |     public static Object invokeStaticMethod(
       576 |             final Class<?> objectClass,
       577 |             final String methodName,
       578 |             Object[] args,
       579 |             Class<?>[] parameterTypes)

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 935)

       930 |      *
       931 |      * @param clazz find method in this class
       932 |      * @param methodName find method with this name
       933 |      * @param parameterTypes find method with compatible parameters
       934 |      * @return The accessible method
>>>    935 |      */
       936 |     public static Method getMatchingAccessibleMethod(
       937 |                                                 final Class<?> clazz,
       938 |                                                 final String methodName,
       939 |                                                 final Class<?>[] parameterTypes) {
       940 |         // trace logging
S9360 (java) on eclipse-jetty - 0 issues removed, 15 issues added - new ruling file

Added jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java (line 61)

(source file not found at this revision: jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/DebugListener.java (line 54)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/DebugListener.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java (line 37)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java (line 48)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java (line 77)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java (line 322)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 192)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/Request.java (line 759)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/Request.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/Request.java (line 768)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/Request.java)

Added jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/AsyncJSON.java (line 71)

(source file not found at this revision: jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/AsyncJSON.java)

Added jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/AsyncJSON.java (line 79)

(source file not found at this revision: jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/AsyncJSON.java)

Added jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/AsyncJSON.java (line 157)

(source file not found at this revision: jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/AsyncJSON.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java (line 43)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java)
S9360 (java) on eclipse-jetty-similar-to-main - 0 issues removed, 8 issues added - new ruling file

Added jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java (line 61)

(source file not found at this revision: jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/DebugListener.java (line 54)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/DebugListener.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java (line 48)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java (line 77)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java (line 322)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 192)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/Request.java (line 759)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/Request.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/Request.java (line 768)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/Request.java)
S9360 (java) on guava - 0 issues removed, 373 issues added - new ruling file

Added src/com/google/common/annotations/GwtCompatible.java (line 63)

        58 |  * <p>Note that a {@code GwtCompatible} type may have some {@link
        59 |  * GwtIncompatible} methods.
        60 |  *
        61 |  * @author Charles Fry
        62 |  * @author Hayward Chan
>>>     63 |  */
        64 | @Retention(RetentionPolicy.CLASS)
        65 | @Target({ ElementType.TYPE, ElementType.METHOD })
        66 | @Documented
        67 | @GwtCompatible
        68 | public @interface GwtCompatible {

Added src/com/google/common/base/CharMatcher.java (line 155)

       150 |   /**
       151 |    * Determines whether a character is a letter or digit according to {@linkplain
       152 |    * Character#isLetterOrDigit(char) Java's definition}.
       153 |    *
       154 |    * @since 19.0 (since 1.0 as constant {@code JAVA_LETTER_OR_DIGIT}).
>>>    155 |    */
       156 |   public static CharMatcher javaLetterOrDigit() {
       157 |     return JavaLetterOrDigit.INSTANCE;
       158 |   }
       159 | 
       160 |   /**

Added src/com/google/common/base/CharMatcher.java (line 165)

       160 |   /**
       161 |    * Determines whether a character is upper case according to {@linkplain
       162 |    * Character#isUpperCase(char) Java's definition}.
       163 |    *
       164 |    * @since 19.0 (since 1.0 as constant {@code JAVA_UPPER_CASE})
>>>    165 |    */
       166 |   public static CharMatcher javaUpperCase() {
       167 |     return JavaUpperCase.INSTANCE;
       168 |   }
       169 | 
       170 |   /**

Added src/com/google/common/base/CharMatcher.java (line 175)

       170 |   /**
       171 |    * Determines whether a character is lower case according to {@linkplain
       172 |    * Character#isLowerCase(char) Java's definition}.
       173 |    *
       174 |    * @since 19.0 (since 1.0 as constant {@code JAVA_LOWER_CASE})
>>>    175 |    */
       176 |   public static CharMatcher javaLowerCase() {
       177 |     return JavaLowerCase.INSTANCE;
       178 |   }
       179 | 
       180 |   /**

Added src/com/google/common/base/Converter.java (line 92)

        87 |  *
        88 |  * @author Mike Ward
        89 |  * @author Kurt Alfred Kluever
        90 |  * @author Gregory Kick
        91 |  * @since 16.0
>>>     92 |  */
        93 | @Beta
        94 | @GwtCompatible
        95 | public abstract class Converter<A, B> implements Function<A, B> {
        96 |   private final boolean handleNullAutomatically;
        97 | 

Added src/com/google/common/base/Converter.java (line 135)

       130 |    * @throws UnsupportedOperationException if backward conversion is not implemented; this should be
       131 |    *     very rare. Note that if backward conversion is not only unimplemented but
       132 |    *     unimplement<i>able</i> (for example, consider a {@code Converter<Chicken, ChickenNugget>}),
       133 |    *     then this is not logically a {@code Converter} at all, and should just implement {@link
       134 |    *     Function}.
>>>    135 |    */
       136 |   protected abstract A doBackward(B b);
       137 | 
       138 |   // API (consumer-side) methods
       139 | 
       140 |   /**

Added src/com/google/common/base/Equivalence.java (line 245)

       240 |    *
       241 |    * <p>Note that this method performs a similar function for equivalences as {@link
       242 |    * com.google.common.collect.Ordering#lexicographical} does for orderings.
       243 |    *
       244 |    * @since 10.0
>>>    245 |    */
       246 |   @GwtCompatible(serializable = true)
       247 |   public final <S extends T> Equivalence<Iterable<S>> pairwise() {
       248 |     // Ideally, the returned equivalence would support Iterable<? extends T>. However,
       249 |     // the need for this is so rare that it's not worth making callers deal with the ugly wildcard.
       250 |     return new PairwiseEquivalence<S>(this);

Added src/com/google/common/base/Equivalence.java (line 325)

       320 |    * System#identityHashCode(Object)} to compute the hash code.  {@link Equivalence#equivalent}
       321 |    * returns {@code true} if {@code a == b}, including in the case that a and b are both null.
       322 |    *
       323 |    * @since 13.0
       324 |    * @since 4.0 (in Equivalences)
>>>    325 |    */
       326 |   public static Equivalence<Object> identity() {
       327 |     return Identity.INSTANCE;
       328 |   }
       329 | 
       330 |   static final class Equals extends Equivalence<Object> implements Serializable {

Added src/com/google/common/base/FinalizableReferenceQueue.java (line 190)

       185 | 
       186 |   /**
       187 |    * Repeatedly dequeues references from the queue and invokes {@link
       188 |    * FinalizableReference#finalizeReferent()} on them until the queue is empty. This method is a
       189 |    * no-op if the background thread was created successfully.
>>>    190 |    */
       191 |   void cleanUp() {
       192 |     if (threadStarted) {
       193 |       return;
       194 |     }
       195 | 

Added src/com/google/common/base/FinalizableSoftReference.java (line 29)

        24 |  * the garbage collector reclaims the referent. This is a simpler alternative to using a {@link
        25 |  * ReferenceQueue}.
        26 |  *
        27 |  * @author Bob Lee
        28 |  * @since 2.0
>>>     29 |  */
        30 | public abstract class FinalizableSoftReference<T> extends SoftReference<T>
        31 |     implements FinalizableReference {
        32 |   /**
        33 |    * Constructs a new finalizable soft reference.
        34 |    *

Added src/com/google/common/base/FinalizableWeakReference.java (line 29)

        24 |  * the garbage collector reclaims the referent. This is a simpler alternative to using a {@link
        25 |  * ReferenceQueue}.
        26 |  *
        27 |  * @author Bob Lee
        28 |  * @since 2.0
>>>     29 |  */
        30 | public abstract class FinalizableWeakReference<T> extends WeakReference<T>
        31 |     implements FinalizableReference {
        32 |   /**
        33 |    * Constructs a new finalizable weak reference.
        34 |    *

Added src/com/google/common/base/Functions.java (line 110)

       105 |    * #forMap(Map, Object)}, which returns a default value in this case.
       106 |    *
       107 |    * <p>Note: if {@code map} is a {@link com.google.common.collect.BiMap BiMap} (or can be one), you
       108 |    * can use {@link com.google.common.collect.Maps#asConverter Maps.asConverter} instead to get a
       109 |    * function that also supports reverse conversion.
>>>    110 |    */
       111 |   public static <K, V> Function<K, V> forMap(Map<K, V> map) {
       112 |     return new FunctionForMapNoDefault<K, V>(map);
       113 |   }
       114 | 
       115 |   private static class FunctionForMapNoDefault<K, V> implements Function<K, V>, Serializable {

Added src/com/google/common/base/Functions.java (line 258)

       253 |   /**
       254 |    * Creates a function that returns the same boolean output as the given predicate for all inputs.
       255 |    *
       256 |    * <p>The returned function is <i>consistent with equals</i> (as documented at {@link
       257 |    * Function#apply}) if and only if {@code predicate} is itself consistent with equals.
>>>    258 |    */
       259 |   public static <T> Function<T, Boolean> forPredicate(Predicate<T> predicate) {
       260 |     return new PredicateFunction<T>(predicate);
       261 |   }
       262 | 
       263 |   /** @see Functions#forPredicate */

Added src/com/google/common/base/Joiner.java (line 331)

       326 | 
       327 |     /**
       328 |      * Appends the string representation of each entry of {@code map}, using the previously
       329 |      * configured separator and key-value separator, to {@code builder}. Identical to {@link
       330 |      * #appendTo(Appendable, Map)}, except that it does not throw {@link IOException}.
>>>    331 |      */
       332 |     public StringBuilder appendTo(StringBuilder builder, Map<?, ?> map) {
       333 |       return appendTo(builder, map.entrySet());
       334 |     }
       335 | 
       336 |     /**

Added src/com/google/common/base/Joiner.java (line 389)

       384 |      * Appends the string representation of each entry in {@code entries}, using the previously
       385 |      * configured separator and key-value separator, to {@code builder}. Identical to {@link
       386 |      * #appendTo(Appendable, Iterable)}, except that it does not throw {@link IOException}.
       387 |      *
       388 |      * @since 10.0
>>>    389 |      */
       390 |     @Beta
       391 |     public StringBuilder appendTo(StringBuilder builder, Iterable<? extends Entry<?, ?>> entries) {
       392 |       return appendTo(builder, entries.iterator());
       393 |     }
       394 | 
S9360 (java) on jboss-ejb3-tutorial - 0 issues removed, 5 issues added - new ruling file

Added partial_deployment_descriptor/src/org/jboss/tutorial/partial_deployment_descriptor/bean/CompleteXMLDDBean.java (line 53)

        48 |    /**
        49 |     * @see CompleteXMLDD#greetWithNotSupportedTransaction(String)
        50 |     * 
        51 |     * Note : The transaction attribute of this method is overriden
        52 |     * through xml file
>>>     53 |     */
        54 |    @TransactionAttribute (TransactionAttributeType.REQUIRES_NEW)
        55 |    public String greetWithNotSupportedTransaction(String name)
        56 |    {
        57 |       String message = "Welcome " + name + ", you are in a method with no transaction supported";
        58 |       logger.info(message);

Added partial_deployment_descriptor/src/org/jboss/tutorial/partial_deployment_descriptor/bean/CompleteXMLDDBean.java (line 64)

        59 |       return message;
        60 |    }
        61 | 
        62 |    /**
        63 |     * @see CompleteXMLDD#greetWithRequiredTransaction(String)
>>>     64 |     */
        65 |    public String greetWithRequiredTransaction(String name)
        66 |    {
        67 |       String message = "Welcome " + name + ", you are in a method with a REQUIRED transaction";
        68 |       logger.info(message);
        69 |       return message;

Added partial_deployment_descriptor/src/org/jboss/tutorial/partial_deployment_descriptor/bean/CompleteXMLDDBean.java (line 74)

        69 |       return message;
        70 |    }
        71 | 
        72 |    /**
        73 |     * @see CompleteXMLDD#sayBye(String)
>>>     74 |     */
        75 |    public String sayBye(String name)
        76 |    {
        77 |       String message = "Bye, " + name + ". Hope to see you again";
        78 |       logger.info(message);
        79 |       return message;
S9360 (java) on sonar-server - 0 issues removed, 10 issues added - new ruling file

Added src/main/java/org/sonar/ce/queue/CeTaskResult.java (line 29)

(source file not found at this revision: src/main/java/org/sonar/ce/queue/CeTaskResult.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/event/EventRepository.java (line 30)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/event/EventRepository.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/MutableMovedFilesRepository.java (line 32)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/MutableMovedFilesRepository.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/formula/Counter.java (line 38)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/formula/Counter.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/MutableQualityGateHolder.java (line 40)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/MutableQualityGateHolder.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/source/DuplicationLineReader.java (line 82)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/source/DuplicationLineReader.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesRepository.java (line 39)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesRepository.java)

Added src/main/java/org/sonar/server/es/IndexingResult.java (line 67)

(source file not found at this revision: src/main/java/org/sonar/server/es/IndexingResult.java)

Added src/main/java/org/sonar/server/setting/ThreadLocalSettings.java (line 51)

(source file not found at this revision: src/main/java/org/sonar/server/setting/ThreadLocalSettings.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistScannerContextStepTest.java (line 85)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistScannerContextStepTest.java)

@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #6006

Please review and merge it into your branch.

romainbrenguier and others added 2 commits August 24, 2026 13:45
…erences

- Resolve unqualified @see/@link names against imports and java.lang
  before flagging them as invalid. Previously, simple names like
  `@see List` or `@see String` were incorrectly prepended with the
  current package name, producing false positives.
- Remove dead-code null/empty guards in extractSeeReferences() and
  resolveReference() flagged by S2589.
- Refactor resolveReference into stripMemberReference + isUnresolvableReference
  for clearer separation of concerns.
- Add compliant test cases for imported types, java.lang types, and
  same-package class references.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Generated with GitHub Actions
@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #6006

Please review and merge it into your branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@romainbrenguier
romainbrenguier marked this pull request as draft August 24, 2026 12:02
@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #6013

Please review and merge it into your branch.

…line

- Fix false positives for inner/nested type references by collecting
  declared types from the compilation unit
- Fix false positives for dotted inner class references (e.g., Map.Entry)
  by trying $ notation with imports and package prefix
- Filter out HTML anchor @see tags (e.g., @see <a href="...">)
- Filter out quoted string @see tags (e.g., @see "text")
- Add missing ruling baseline for eclipse-jetty-similar-to-main

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #6015

Please review and merge it into your branch.

romainbrenguier and others added 3 commits August 24, 2026 14:58
…ndling

- Remove dead-code condition in setContext (S2589: getTree() is non-null)
- Hoist Tree.Kind[] allocation to static final constant
- Fix multi-level nested inner class resolution (e.g., Outer.Inner.Deep)
- Convert skip tests to parameterized test (S5976)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
🤖 Generated with GitHub Actions
@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #6017

Please review and merge it into your branch.

…for S9360

Remove false positives from the ruling baseline that were eliminated
by the improved import resolution and inner class handling in S9360.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #6017

Please review and merge it into your branch.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@gitar-bot

gitar-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 5 resolved / 5 findings

Implements rule S9360 to detect Javadoc references pointing to non-existent types, addressing dead code, false positives on simple names and nested inner classes, and allocation inefficiencies.

✅ 5 resolved
Bug: Relative @see reference resolution is dead code

📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:144-158 📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:39-51
In resolveReference, if (reference.contains(".")) return reference; already handles every dotted name, so the following if (!reference.contains(".")) return null; is always true for the remaining (dot-free) names and returns null. The package-qualification branch (currentPackage + "." + reference) is therefore unreachable, so bare same-package references such as @see ExistentClass are never validated — contradicting the HTML doc which advertises "Relative references to classes that don't exist in the current package." Additionally, even if reached, currentPackage = pkg.toString() is unreliable because PackageDeclarationTree does not override toString() (it returns Object's default, not the package name). Fix the control flow to fall through to package qualification for simple names, and derive the package name from the tree's identifiers/tokens rather than toString(). Also add a test case for a relative reference so this gap is covered.

Bug: SEE_ANCHOR_PATTERN matches {@see, not a real tag

📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:37 📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:111-117
SEE_ANCHOR_PATTERN looks for the inline form {@see ...}, but @see is only a block tag; the inline cross-reference tag in Javadoc is {@link ...}/{@linkplain ...}. As written this pattern never matches valid Javadoc (and the SEE_TAG_PATTERN would in any case also match the @see substring inside braces, capturing a trailing }). If inline link checking is intended, match \{@link(?:plain)?\s++... and strip the trailing brace; otherwise remove the anchor pattern.

Bug: Simple-name @see refs to imported/java.lang types flagged as invalid

📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:150-160 📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:93-96
Removing the !reference.contains(".") guard activated relative resolution: a simple name like @see List (imported from java.util) or @see String is resolved to <currentPackage>.List, and Sema.getClassType only resolves fully qualified names — it ignores compilation-unit imports and java.lang. So valid simple-name references to imported or java.lang types are reported as invalid (false positives), and references defined outside the current package are never resolvable. Consider resolving simple names against the compilation unit's imports/java.lang (or skipping unqualified names entirely) rather than blindly prefixing the current package, and add a compliant test for @see List where List is imported.

Edge Case: Multi-level nested inner refs (a.b.Outer.Inner.Deep) still flagged

📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:199-213
isUnresolvableInnerClassReference only converts a single dot to $ per iteration, so a deeply-nested reference like pkg.Outer.Inner.Deep is only ever tested as pkg.Outer.Inner$Deep or pkg.Outer$Inner.Deep, never the correct pkg.Outer$Inner$Deep. Such valid references produce false positives. If deeply nested references are a concern, replace all trailing dots (after the outer type) with $ progressively, or resolve the outer type first and then walk the inner segments.

Quality: New Tree.Kind[] allocated on every recursive call

📄 java-checks/src/main/java/org/sonar/java/checks/JavadocReferencesExistingSymbolsCheck.java:93-96
collectDeclaredTypesFromTree calls Tree.Kind.CLASS_KINDS.toArray(new Tree.Kind[0]) on every invocation, allocating a fresh array for each type/member recursively. Hoist the converted array into a private static final Tree.Kind[] constant (or check Tree.Kind.CLASS_KINDS.contains(tree.kind())) to avoid per-node allocation.

Implementation Status ◻️ 0 of 1 objectives covered
◻️ SONARJAVA-6825 - 0 of 1 objectives covered

The PR implements rule S9360, but the rule implementation and tests are for checking Javadoc references existing types rather than simplifying constant expressions and comparisons as stated in the issue description.

Other objectives on this issue, possibly covered elsewhere:

  • ◻️ Implement rule S9360: Constant expressions and comparisons should be simplified
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown
Contributor

@romainbrenguier

Copy link
Copy Markdown
Contributor Author

This implements the wrong rule

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 this pull request may close these issues.

1 participant