SONARJAVA-6976 Clear check state after file analysis

[romainbrenguier]

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.

Agent workflow

Addressed review comments in pr_report_6144.md using uv run address_reviews.py pr_report_6144.md

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document [MAJOR] Refactor the code of the lambda to have only one invocation possibly throwing a runtime exception. (component: java-frontend/src/test/java/org/sonar/java/model/VisitorsBridgeTest.java, line: 187)." -a "[MAJOR] Refactor the code of the lambda to have only one invocation possibly throwing a runtime exception. (component: java-frontend/src/test/java/org/sonar/java/model/VisitorsBridgeTest.java, line: 187)" -g "claude"

[hashicorp-vault-sonar-prod]

SONARJAVA-6976

[github-actions]

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

Please review and merge it into your branch.

[github-actions]

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

Please review and merge it into your branch.

[github-actions]

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)

✅ All code review findings resolved.

_{Configure merge blocking}

[github-actions]

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

Please review and merge it into your branch.

[aurelien-coet-sonarsource]

Maybe we could also document somewhere (e.g. in .claude/commands/project-level-rules.md) that checks which use collections to store some state should override clearState to clear it between files ?

[aurelien-coet-sonarsource]

If a class overrides leaveFile and doesn't call super.leaveFile(context) or clearState(), then the state will not be cleared, even if clearState is overriden to do so.
Furthermore, leaveFile is called inside a try .. catch block in VisitorsBridge.scanFile, after calling other methods that could throw, so if they do, then leaveFile is never called and the state is never cleared.
It may make more sense to directly call clearState on every visitor in the finally block inside VisitorsBridge.scanFile. WDYT ?

[romainbrenguier]

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...

This comment is outdated. The PR should not update ruling results.

[sonarqube-next]

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

[aurelien-coet-sonarsource]

Two comments are still unaddressed.

[github-actions]

This PR is stale because it has been open 7 days with no activity. If there is no activity in the next 7 days it will be closed automatically

[gitar-bot]

Code Review ✅ Approved 14 closed / 14 findings

🟡 Medium risk · Visitor lifecycle now clears mutable check state before and after file analysis

Refactors check state management to guarantee cleanup at file analysis boundaries, addressing state carryover issues (S1166 exclusion list wiped, S6539 imports not recomputed, S120 project accumulator retained across files). Introduces clearState() hook and finally blocks for reliable per-file reporting, with test coverage extended to module-end cleanup. All regressions resolved.

✅ 14 closed

✅ 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 9 more closed 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]

Quality Gate passed

Issues
0 New issues
1 Fixed issue
0 Accepted issues

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

See analysis details on SonarQube

[aurelien-coet-sonarsource]

Here, if any of the visitors throws an exception while clearing its state, any other visitor that hasn't cleared it yet will be skipped and its state won't be cleaned up.
Maybe it would be better to catch exceptions inside the for loop and log them, but still iterate over the remaining checks without interruptions to guarantee cleanup. WDYT ?

[aurelien-coet-sonarsource]

Why is this method needed ? Couldn't we directly call visitor.clearState ?