SONARJAVA-6976 Clear check state after file analysis by romainbrenguier · Pull Request #6144 · SonarSource/sonar-java

romainbrenguier · 2026-09-15T08:17:45Z

Summary

Clear collection-backed check state at file or module-analysis boundaries.
Use finally blocks around per-file reporting where appropriate.
Extend the ArchUnit lifecycle test to cover module-end cleanup.

Validation

CheckStateCleanupTest passes with zero violations.
102 focused java-checks tests pass.
Compilation and whitespace checks pass.

…lived processes Move cache clearing (ifStatementCache, firstNullCheckCache, safeSymbols) from the start of scanFile to a finally block, so the last scanned file's AST is not retained for the lifetime of the plugin classloader in IDE sessions (SonarLint). Co-Authored-By: Claude Opus 4.6 <[email protected]>

hashicorp-vault-sonar-prod · 2026-09-15T08:18:06Z

SONARJAVA-6976

github-actions · 2026-09-15T08:34:18Z

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

Please review and merge it into your branch.

github-actions · 2026-09-15T09:08:22Z

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

Please review and merge it into your branch.

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

github-actions · 2026-09-15T09:17:54Z

Ruling Diff Summary

Detected changes in 5 rule files: 0 issues removed, 75 issues added.

S1166 (java) on commons-beanutils - 0 issues removed, 23 issues added

Added src/main/java/org/apache/commons/beanutils2/BeanComparator.java (line 174)

       169 |             throw new RuntimeException( "IllegalAccessException: " + iae.toString() );
       170 |         }
       171 |         catch ( final InvocationTargetException ite ) {
       172 |             throw new RuntimeException( "InvocationTargetException: " + ite.toString() );
       173 |         }
>>>    174 |         catch ( final NoSuchMethodException nsme ) {
       175 |             throw new RuntimeException( "NoSuchMethodException: " + nsme.toString() );
       176 |         }
       177 |     }
       178 | 
       179 |     /**

Added src/main/java/org/apache/commons/beanutils2/BeanMap.java (line 826)

       821 |         try {
       822 |             final Constructor<?> constructor = newType.getConstructor( value.getClass() );
       823 |             final Object[] arguments = { value };
       824 |             return constructor.newInstance( arguments );
       825 |         }
>>>    826 |         catch ( final NoSuchMethodException e ) {
       827 |             // try using the transformers
       828 |             final Transformer transformer = getTypeTransformer( newType );
       829 |             if ( transformer != null ) {
       830 |                 return transformer.transform( value );
       831 |             }

Added src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java (line 284)

       279 |                     getPropertyUtils().isWriteable(dest, name)) {
       280 |                     try {
       281 |                         final Object value =
       282 |                             getPropertyUtils().getSimpleProperty(orig, name);
       283 |                         copyProperty(dest, name, value);
>>>    284 |                     } catch (final NoSuchMethodException e) {
       285 |                         // Should not happen
       286 |                     }
       287 |                 }
       288 |             }
       289 |         }

Added src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java (line 363)

       358 |         final Resolver resolver = getPropertyUtils().getResolver();
       359 |         while (resolver.hasNested(name)) {
       360 |             try {
       361 |                 target = getPropertyUtils().getProperty(target, resolver.next(name));
       362 |                 name = resolver.remove(name);
>>>    363 |             } catch (final NoSuchMethodException e) {
       364 |                 return; // Skip this property setter
       365 |             }
       366 |         }
       367 |         if (log.isTraceEnabled()) {
       368 |             log.trace("    Target bean = " + target);

Added src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java (line 394)

       389 |                 descriptor =
       390 |                     getPropertyUtils().getPropertyDescriptor(target, name);
       391 |                 if (descriptor == null) {
       392 |                     return; // Skip this property setter
       393 |                 }
>>>    394 |             } catch (final NoSuchMethodException e) {
       395 |                 return; // Skip this property setter
       396 |             }
       397 |             type = descriptor.getPropertyType();
       398 |             if (type == null) {
       399 |                 // Most likely an indexed setter on a POJB only

Added src/main/java/org/apache/commons/beanutils2/ConstructorUtils.java (line 307)

       302 |         final Class<?>[] parameterTypes) {
       303 | 
       304 |         try {
       305 |             return getAccessibleConstructor(
       306 |                 klass.getConstructor(parameterTypes));
>>>    307 |         } catch (final NoSuchMethodException e) {
       308 |             return null;
       309 |         }
       310 |     }
       311 | 
       312 |     /**

Added src/main/java/org/apache/commons/beanutils2/ConstructorUtils.java (line 396)

       391 |             } catch (final SecurityException se) {
       392 |                 /* SWALLOW, if workaround fails don't fret. */
       393 |             }
       394 |             return ctor;
       395 | 
>>>    396 |         } catch (final NoSuchMethodException e) { /* SWALLOW */
       397 |         }
       398 | 
       399 |         // search through all methods
       400 |         final int paramSize = parameterTypes.length;
       401 |         final Constructor<?>[] ctors = clazz.getConstructors();

Added src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java (line 488)

       483 |                 }
       484 |                 try {
       485 |                     m = clazz.getMethod(methodName, paramTypes);
       486 |                     // Un-comment following line for testing
       487 |                     // System.out.println("Recreated Method " + methodName + " for " + className);
>>>    488 |                 } catch (final NoSuchMethodException e) {
       489 |                     throw new RuntimeException("Method " + methodName + " for " +
       490 |                             className + " could not be reconstructed - method not found");
       491 |                 }
       492 |                 methodRef = new SoftReference<>(m);
       493 |             }

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

       733 | 
       734 |             method =  getAccessibleMethod
       735 |                     (clazz, clazz.getMethod(methodName, parameterTypes));
       736 |             cacheMethod(md, method);
       737 |             return method;
>>>    738 |         } catch (final NoSuchMethodException e) {
       739 |             return null;
       740 |         }
       741 |     }
       742 | 
       743 |     /**

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

       838 |         Class<?> parentClazz = clazz.getSuperclass();
       839 |         while (parentClazz != null) {
       840 |             if (Modifier.isPublic(parentClazz.getModifiers())) {
       841 |                 try {
       842 |                     return parentClazz.getMethod(methodName, parameterTypes);
>>>    843 |                 } catch (final NoSuchMethodException e) {
       844 |                     return null;
       845 |                 }
       846 |             }
       847 |             parentClazz = parentClazz.getSuperclass();
       848 |         }

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

       882 | 
       883 |                 // Does the method exist on this interface?
       884 |                 try {
       885 |                     method = interfaces[i].getDeclaredMethod(methodName,
       886 |                             parameterTypes);
>>>    887 |                 } catch (final NoSuchMethodException e) {
       888 |                     /* Swallow, if no method is found after the loop then this
       889 |                      * method returns null.
       890 |                      */
       891 |                 }
       892 |                 if (method != null) {

Added src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java (line 1346)

      1341 |                 nestedBean = getProperty(bean, next);
      1342 |             } catch (final IllegalAccessException e) {
      1343 |                 return false;
      1344 |             } catch (final InvocationTargetException e) {
      1345 |                 return false;
>>>   1346 |             } catch (final NoSuchMethodException e) {
      1347 |                 return false;
      1348 |             }
      1349 |             if (nestedBean == null) {
      1350 |                 throw new NestedNullException
      1351 |                         ("Null property value for '" + next +

Added src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java (line 1392)

      1387 |             return false;
      1388 |         } catch (final IllegalAccessException e) {
      1389 |             return false;
      1390 |         } catch (final InvocationTargetException e) {
      1391 |             return false;
>>>   1392 |         } catch (final NoSuchMethodException e) {
      1393 |             return false;
      1394 |         }
      1395 | 
      1396 |     }
      1397 |

Added src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java (line 1435)

      1430 |                 nestedBean = getProperty(bean, next);
      1431 |             } catch (final IllegalAccessException e) {
      1432 |                 return false;
      1433 |             } catch (final InvocationTargetException e) {
      1434 |                 return false;
>>>   1435 |             } catch (final NoSuchMethodException e) {
      1436 |                 return false;
      1437 |             }
      1438 |             if (nestedBean == null) {
      1439 |                 throw new NestedNullException
      1440 |                         ("Null property value for '" + next +

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtilsBean.java (line 675)

       670 |         final Resolver resolver = getPropertyUtils().getResolver();
       671 |         while (resolver.hasNested(name)) {
       672 |             try {
       673 |                 target = getPropertyUtils().getProperty(target, resolver.next(name));
       674 |                 name = resolver.remove(name);
>>>    675 |             } catch (final NoSuchMethodException e) {
       676 |                 return; // Skip this property setter
       677 |             }
       678 |         }
       679 |         if (log.isTraceEnabled()) {
       680 |             log.trace("    Target bean = " + target);

S1166 (java) on eclipse-jetty - 0 issues removed, 9 issues added

Added jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java (line 726)

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

Added jetty-util/src/main/java/org/eclipse/jetty/util/LeakDetector.java (line 153)

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

Added jetty-util/src/main/java/org/eclipse/jetty/util/ProcessorUtils.java (line 41)

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

Added jetty-util/src/main/java/org/eclipse/jetty/util/SharedBlockingCallback.java (line 74)

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

Added jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java (line 829)

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

Added jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java (line 841)

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

Added jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java (line 394)

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

Added jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java (line 839)

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

Added jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java (line 955)

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

S1166 (java) on guava - 0 issues removed, 28 issues added

Added src/com/google/common/base/internal/Finalizer.java (line 132)

       127 |     while (true) {
       128 |       try {
       129 |         if (!cleanUp(queue.remove())) {
       130 |           break;
       131 |         }
>>>    132 |       } catch (InterruptedException e) {
       133 |         // ignore
       134 |       }
       135 |     }
       136 |   }
       137 |

Added src/com/google/common/cache/CacheBuilderSpec.java (line 441)

       436 |                     key, value));
       437 |         }
       438 | 
       439 |         long duration = Long.parseLong(value.substring(0, value.length() - 1));
       440 |         parseDuration(spec, duration, timeUnit);
>>>    441 |       } catch (NumberFormatException e) {
       442 |         throw new IllegalArgumentException(
       443 |             format("key %s value set to %s, must be integer", key, value));
       444 |       }
       445 |     }
       446 |   }

Added src/com/google/common/collect/ComputingConcurrentHashMap.java (line 333)

       328 |         try {
       329 |           synchronized (this) {
       330 |             while (computedReference == UNSET) {
       331 |               try {
       332 |                 wait();
>>>    333 |               } catch (InterruptedException ie) {
       334 |                 interrupted = true;
       335 |               }
       336 |             }
       337 |           }
       338 |         } finally {

Added src/com/google/common/collect/Queues.java (line 317)

       312 |           E e; // written exactly once, by a successful (uninterrupted) invocation of #poll
       313 |           while (true) {
       314 |             try {
       315 |               e = q.poll(deadline - System.nanoTime(), TimeUnit.NANOSECONDS);
       316 |               break;
>>>    317 |             } catch (InterruptedException ex) {
       318 |               interrupted = true; // note interruption and retry
       319 |             }
       320 |           }
       321 |           if (e == null) {
       322 |             break; // we already waited enough, and there are no more elements in sight

Added src/com/google/common/net/HostAndPort.java (line 197)

       192 |       // Try to parse the whole port string as a number.
       193 |       // JDK7 accepts leading plus signs. We don't want to.
       194 |       checkArgument(!portString.startsWith("+"), "Unparseable port number: %s", hostPortString);
       195 |       try {
       196 |         port = Integer.parseInt(portString);
>>>    197 |       } catch (NumberFormatException e) {
       198 |         throw new IllegalArgumentException("Unparseable port number: " + hostPortString);
       199 |       }
       200 |       checkArgument(isValidPort(port), "Port number out of range: %s", hostPortString);
       201 |     }
       202 |

Added src/com/google/common/net/InetAddresses.java (line 215)

       210 |     byte[] bytes = new byte[IPV4_PART_COUNT];
       211 |     try {
       212 |       for (int i = 0; i < bytes.length; i++) {
       213 |         bytes[i] = parseOctet(address[i]);
       214 |       }
>>>    215 |     } catch (NumberFormatException ex) {
       216 |       return null;
       217 |     }
       218 | 
       219 |     return bytes;
       220 |   }

Added src/com/google/common/net/InetAddresses.java (line 279)

       274 |         rawBytes.putShort((short) 0);
       275 |       }
       276 |       for (int i = partsLo; i > 0; i--) {
       277 |         rawBytes.putShort(parseHextet(parts[parts.length - i]));
       278 |       }
>>>    279 |     } catch (NumberFormatException ex) {
       280 |       return null;
       281 |     }
       282 |     return rawBytes.array();
       283 |   }
       284 |

Added src/com/google/common/primitives/Doubles.java (line 641)

       636 |     if (FLOATING_POINT_PATTERN.matcher(string).matches()) {
       637 |       // TODO(lowasser): could be potentially optimized, but only with
       638 |       // extensive testing
       639 |       try {
       640 |         return Double.parseDouble(string);
>>>    641 |       } catch (NumberFormatException e) {
       642 |         // Double.parseDouble has changed specs several times, so fall through
       643 |         // gracefully
       644 |       }
       645 |     }
       646 |     return null;

Added src/com/google/common/primitives/Floats.java (line 617)

       612 |     if (Doubles.FLOATING_POINT_PATTERN.matcher(string).matches()) {
       613 |       // TODO(lowasser): could be potentially optimized, but only with
       614 |       // extensive testing
       615 |       try {
       616 |         return Float.parseFloat(string);
>>>    617 |       } catch (NumberFormatException e) {
       618 |         // Float.parseFloat has changed specs several times, so fall through
       619 |         // gracefully
       620 |       }
       621 |     }
       622 |     return null;

Added src/com/google/common/reflect/ClassPath.java (line 369)

       364 |       if (classpathAttribute != null) {
       365 |         for (String path : CLASS_PATH_ATTRIBUTE_SEPARATOR.split(classpathAttribute)) {
       366 |           URL url;
       367 |           try {
       368 |             url = getClassPathEntry(jarFile, path);
>>>    369 |           } catch (MalformedURLException e) {
       370 |             // Ignore bad entry
       371 |             logger.warning("Invalid Class-Path entry: " + path);
       372 |             continue;
       373 |           }
       374 |           if (url.getProtocol().equals("file")) {

Added src/com/google/common/reflect/Types.java (line 571)

       566 |       }
       567 |       @Override String typeName(Type type) {
       568 |         try {
       569 |           Method getTypeName = Type.class.getMethod("getTypeName");
       570 |           return (String) getTypeName.invoke(type);
>>>    571 |         } catch (NoSuchMethodException e) {
       572 |           throw new AssertionError("Type.getTypeName should be available in Java 8");
       573 |         } catch (InvocationTargetException e) {
       574 |           throw new RuntimeException(e);
       575 |         } catch (IllegalAccessException e) {
       576 |           throw new RuntimeException(e);

Added src/com/google/common/util/concurrent/Monitor.java (line 399)

       394 |     try {
       395 |       final long startTime = System.nanoTime();
       396 |       for (long remainingNanos = timeoutNanos;;) {
       397 |         try {
       398 |           return lock.tryLock(remainingNanos, TimeUnit.NANOSECONDS);
>>>    399 |         } catch (InterruptedException interrupt) {
       400 |           interrupted = true;
       401 |           remainingNanos = remainingNanos(startTime, timeoutNanos);
       402 |         }
       403 |       }
       404 |     } finally {

Added src/com/google/common/util/concurrent/Monitor.java (line 565)

       560 |             if (lock.tryLock(remainingNanos, TimeUnit.NANOSECONDS)) {
       561 |               break;
       562 |             } else {
       563 |               return false;
       564 |             }
>>>    565 |           } catch (InterruptedException interrupt) {
       566 |             interrupted = true;
       567 |             remainingNanos = remainingNanos(startTime, timeoutNanos);
       568 |           }
       569 |         }
       570 |       }

Added src/com/google/common/util/concurrent/Monitor.java (line 589)

       584 |                 remainingNanos = remainingNanos(startTime, timeoutNanos);
       585 |               }
       586 |               satisfied = awaitNanos(guard, remainingNanos, signalBeforeWaiting);
       587 |             }
       588 |             return satisfied;
>>>    589 |           } catch (InterruptedException interrupt) {
       590 |             interrupted = true;
       591 |             signalBeforeWaiting = false;
       592 |           }
       593 |         }
       594 |       } finally {

Added src/com/google/common/util/concurrent/MoreExecutors.java (line 215)

       210 |             // is undefined in shutdown hooks.
       211 |             // This is because the logging code installs a shutdown hook of its
       212 |             // own. See Cleaner class inside {@link LogManager}.
       213 |             service.shutdown();
       214 |             service.awaitTermination(terminationTimeout, timeUnit);
>>>    215 |           } catch (InterruptedException ignored) {
       216 |             // We're shutting down anyway, so just ignore.
       217 |           }
       218 |         }
       219 |       }));
       220 |     }

S1166 (java) on jboss-ejb3-tutorial - 0 issues removed, 1 issues added

Added enterprise_app_ejb_injection/webapp/src/main/java/org/jboss/tutorial/enterprise_app_ejb_injection/servlet/CalculatorActionServlet.java (line 80)

        75 |       int op1;
        76 |       int op2;
        77 |       try {
        78 |          op1 = Integer.parseInt(req.getParameter("op1"));
        79 |          op2 = Integer.parseInt(req.getParameter("op2"));
>>>     80 |       } catch (NumberFormatException nfe)
        81 |       {
        82 |          printMessage(resp, "Only integers are allowed in input");
        83 |          return;
        84 |       }
        85 |       String action = req.getParameter("action");

S1166 (java) on sonar-server - 0 issues removed, 14 issues added

Added src/main/java/org/sonar/server/component/ws/ProjectMeasuresQueryFactory.java (line 150)

(source file not found at this revision: src/main/java/org/sonar/server/component/ws/ProjectMeasuresQueryFactory.java)

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

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

Added src/main/java/org/sonar/server/computation/task/projectanalysis/step/PeriodResolver.java (line 182)

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

Added src/main/java/org/sonar/server/debt/DebtRulesXMLImporter.java (line 158)

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

Added src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java (line 145)

(source file not found at this revision: src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java)

Added src/main/java/org/sonar/server/platform/ServerIdManager.java (line 82)

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

Added src/main/java/org/sonar/server/project/ws/IndexAction.java (line 112)

(source file not found at this revision: src/main/java/org/sonar/server/project/ws/IndexAction.java)

Added src/main/java/org/sonar/server/property/ws/IndexAction.java (line 125)

(source file not found at this revision: src/main/java/org/sonar/server/property/ws/IndexAction.java)

Added src/main/java/org/sonar/server/qualitygate/ws/DeselectAction.java (line 96)

(source file not found at this revision: src/main/java/org/sonar/server/qualitygate/ws/DeselectAction.java)

Added src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWs.java (line 92)

(source file not found at this revision: src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWs.java)

Added src/main/java/org/sonar/server/qualitygate/ws/SelectAction.java (line 134)

(source file not found at this revision: src/main/java/org/sonar/server/qualitygate/ws/SelectAction.java)

Added src/main/java/org/sonar/server/util/FloatTypeValidation.java (line 40)

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

Added src/main/java/org/sonar/server/util/IntegerTypeValidation.java (line 40)

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

Added src/main/java/org/sonar/server/util/LongTypeValidation.java (line 39)

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

- Remove no-op pre-clear guards before reassignment in NestedIfStatementsCheck and TypeParametersShadowingCheck - Revert unnecessary mutable copy in DepthOfInheritanceTreeCheck (filteredPatterns is a property cache, not AST state) - Fix indentation in ClassImportCouplingCheck finally block - Fix MissingPackageInfoCheckTest: remove assertion on internal state that is now properly cleared in endOfAnalysis Co-Authored-By: Claude Opus 4.6 <[email protected]>

…ghten test - RedundantTypeCastCheck: set warnings = null instead of calling .clear() on the unmodifiable list, which threw UnsupportedOperationException - ClassImportCouplingCheck: remove redundant guarded cleanup blocks that can never execute; simplify to try/finally with null assignments - CheckStateCleanupTest: fix isPropertyCache returning true on empty stream (allMatch vacuous truth) by requiring non-empty value types Co-Authored-By: Claude Opus 4.6 <[email protected]>

✅ All code review findings resolved.

_{Configure merge blocking}

github-actions · 2026-09-15T09:53:05Z

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

Please review and merge it into your branch.

…6 ruling Add a protected clearState() hook to IssuableSubscriptionVisitor, called automatically from setContext() before each file analysis. Checks that need to clear per-file state now simply override clearState() instead of duplicating the setContext/leaveFile boilerplate in every file. This eliminates ~210 lines of duplicated code across 14 checks. Also reverts the S1166 ruling file updates that were based on the bug where exceptions/exceptionIdentifiers were incorrectly cleared. Co-Authored-By: Claude Opus 4.6 <[email protected]>

…dant override - Add clearState() call in IssuableSubscriptionVisitor.leaveFile() so state is released after the last file analysis, not just before the next one - Remove redundant setContext() override in UnusedPrivateFieldCheck since the base class already calls clearState() in setContext() - Remove unused JavaFileScannerContext import in UnreachableCatchCheck (SQ QG) - Relax ArchUnit test to accept field cleared by any lifecycle method, since clearState() is called from both setContext() and leaveFile() Co-Authored-By: Claude Opus 4.6 <[email protected]>

gitar-bot · 2026-09-15T10:32:39Z

Code Review ✅ Approved 13 resolved / 13 findings

Clears collection-backed check state at file and module-analysis boundaries to prevent AST retention in long-lived processes. Resolves false positives from S1166 exclusion list wiping, S6539 import recomputation failures, S120 project-level accumulator clearing at file start, and several other check state lifecycle issues including redundant clears, dead code, and missing cleanup calls.

✅ 13 resolved

✅ Bug: S1166: exclusion list wiped after first file (false positives)

📄 java-checks/src/main/java/org/sonar/java/checks/CatchUsesExceptionWithContextCheck.java:119-127 📄 java-checks/src/main/java/org/sonar/java/checks/CatchUsesExceptionWithContextCheck.java:239-253 📄 java-checks/src/main/java/org/sonar/java/checks/CatchUsesExceptionWithContextCheck.java:114-119 📄 java-checks/src/main/java/org/sonar/java/checks/CatchUsesExceptionWithContextCheck.java:126-131 📄 java-checks/src/main/java/org/sonar/java/checks/CatchUsesExceptionWithContextCheck.java:245-259
exceptions and exceptionIdentifiers are memoized rule-property caches guarded by if (exceptions == null) (lines 239-258). The new finally calls clear() on them without resetting them to null, so from the second file of an analysis onward getExceptions()/getExceptionIdentifiers() return empty sets and isExcludedType never matches — every catch (InterruptedException|NumberFormatException|ParseException|...) that is not logged or rethrown is now reported, i.e. new false positives on S1166 for all files after the first. Existing tests use one CheckVerifier file per instance so CI will not catch it. These two sets hold only Strings (no AST), so the cleanup brings no memory benefit: drop the clears (or set the fields to null).

✅ Bug: S6539: imports never recomputed after first file

📄 java-checks/src/main/java/org/sonar/java/checks/design/ClassImportCouplingCheck.java:53-67 📄 java-checks/src/main/java/org/sonar/java/checks/design/ClassImportCouplingCheck.java:82-95
visitClass builds imports/secondaryLocations only once per file via if (imports == null) { ... } (lines 82-95), which is why the old code set both fields to null at the end of scanFile. The new code only calls clear() and never nulls them, so for the second and every subsequent file imports stays non-null and empty: the coupling count imports.size() + types.size() loses all import contributions and secondary locations are missing, producing false negatives for S6539 on all files after the first. Restore the null assignments (clearing first is fine for releasing the AST references).

✅ Bug: S120: project-level accumulator cleared at start of each file

📄 java-checks/src/main/java/org/sonar/java/checks/naming/BadPackageNameCheck.java:50-62 📄 java-checks/src/main/java/org/sonar/java/checks/naming/BadPackageNameCheck.java:65-75 📄 java-checks/src/main/java/org/sonar/java/checks/naming/BadPackageNameCheck.java:86-92
badPackageNames is a project-level accumulator: scanFile/scanWithoutParsing add to it and endOfAnalysis reports one project issue per collected name (it already clears the set there). Adding badPackageNames.clear() at the top of scanFile discards every name collected from previously scanned files — including names restored from the cache by scanWithoutParsing, which runs before the parsing phase — so only the last-scanned file's package can ever be reported. A project with several badly named packages now yields a single issue instead of one per package. Remove the clear from scanFile.

✅ Quality: CheckStateCleanupTest does not enforce what it claims

📄 java-checks/src/test/java/org/sonar/java/checks/CheckStateCleanupTest.java:53-67 📄 java-checks/src/test/java/org/sonar/java/checks/CheckStateCleanupTest.java:82-96 📄 java-checks/src/test/java/org/sonar/java/checks/CheckStateCleanupTest.java:106-120
The new ArchUnit rule cannot detect the omissions it is meant to guard: (1) clearsField only requires that the lifecycle method touches the field and contains some clear() call on any Collection/Map, so removing e.g. settersToReport.clear() from CookieHttpOnlyCheck.leaveFile still passes because the other clears remain; (2) lifecycleMethods returns an empty set for checks that declare none of setContext/leaveFile/endOfAnalysis (and scanFile is final in IssuableSubscriptionVisitor), so the condition is vacuously satisfied for state-holding checks such as UselessImportCheck (imports, importsNames, duplicatedImports), SelfAssignmentCheck (warnings) and VolatileVariablesOperationsCheck (visitedUnaryExpressions) — exactly the AST-retaining fields this PR targets. "Zero violations" therefore does not demonstrate cleanup, and it drives no-op clears elsewhere in the diff. Tie the clear call to the specific field (e.g. require a clear() whose target owner matches the field's raw type and that is invoked on that field access) and report a violation when a check has collection state but no lifecycle method at all.

✅ Performance: IndentationCheck copies all file lines just to clear them

📄 java-checks/src/main/java/org/sonar/java/checks/IndentationCheck.java:70-76
fileLines = new ArrayList<>(context.getFileLines()) allocates a per-file copy of the whole line list only so that fileLines.clear() does not mutate the context-owned list. The copy holds the same String instances, so clearing it reclaims nothing that fileLines = null would not, while adding an O(lines) allocation for every analyzed file — the opposite of this PR's memory goal.

...and 8 more resolved from earlier reviews

Review coverage

Functional validation No results

Rules No rules evaluated

Auto-approval Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

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 · 2026-09-15T10:41:07Z

Quality Gate passed

Issues
0 New issues
1 Fixed issue
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

romainbrenguier and others added 4 commits September 14, 2026 17:20

fix: clear safeSymbols to prevent memory leak across files

1a0a1d9

Add check state cleanup architecture test

1ded950

Clear check state after file analysis

da09bc0

hashicorp-vault-sonar-prod Bot changed the title ~~Clear check state after file analysis~~ SONARJAVA-6976 Clear check state after file analysis Sep 15, 2026

Reset check state before file analysis

202506e

This comment has been minimized.

Sign in to view