Skip to content

RC-451 Migrate Java ruling expectations to SARIF - #6445

Open
romainbrenguier wants to merge 12 commits into
masterfrom
romain/rc-451
Open

romainbrenguier wants to merge 12 commits into
masterfrom
romain/rc-451

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Upgrade LITS to the latest promoted master build, 0.13.0.16212 (SonarSource/sonar-lits@f1c22c6).
  • Select .sarif expectations for subset ruling runs.
  • Delete 1,279 legacy JSON ruling expectations; the existing automatic ruling-update PR will regenerate SARIF baselines.
  • Reference all core-languages-tooling-public actions from master.
  • Trigger ruling diff comments for SARIF changes and remove the obsolete ruling-root input.

The LITS upgrade, expectation deletion, and action updates are separate commits.

Validation

  • git diff --check passes.
  • Ruling integration tests intentionally left to CI (not run locally).
  • DEEP Vortex analysis completed: no findings on changed lines. Three existing java:S3457 findings remain on untouched lines 558–560 of JavaRulingTest.java: Format specifiers should be used instead of string concatenation.
  • A separate stacked draft PR will exercise SARIF diff comments without mixing smoke-test expectations into this migration.

Summary by Gitar

  • Ruling baselines:
    • Added its/ruling/src/test/resources/expected/.gitkeep to preserve the empty expected root for SARIF regeneration
    • upload-actual now creates the expected directory and uses --new-file, so diffs generate when baselines do not exist yet
  • SARIF migration:
    • Added SARIF baselines and removed legacy JSON ruling expectations for subset ruling runs
    • ruling-diff-comment workflow now triggers on SARIF changes
  • LITS upgrade:
    • JavaRulingTest now loads sonar-lits-plugin 0.13.0.16223 and selects .sarif expectations
  • Diff rendering:
    • upload-actual raises Node heap to 8192 MB and caps diff2html at 2000 changes to bound memory
  • Documentation:
    • new-rule skill now collects TP/FP findings from SARIF diffs instead of JSON expectations

This will update automatically on new commits.

Agent workflow

Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR updated using `uv run update_with_claude.py --prompt "Implement the action plan described in the document Update the version of the lits plugin used in this PR now that sonar-lits has been released, version: sonar-lits:0.13.0.16223." -a "Update the version of the lits plugin used in this PR now that sonar-lits has been released, version: sonar-lits:0.13....

PR updated using uv run update_with_claude.py --prompt "Rebase on origin/master, fixing conflicts" -a "None" -g "codex"

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document /tmp/action_plan_romain/rc-451.txt." -a "/tmp/action_plan_romain/rc-451.txt" -g "codex"

Iterated on the PR with uv run ci_loop.py for 1 iterations.
✔️ The PR is now ready for review.

PR updated using `uv run update_with_claude.py --prompt "Implement the action plan described in the document Some docs needs to be updated.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

RC-451

@romainbrenguier

Copy link
Copy Markdown
Contributor Author

Stacked smoke-test draft: #6446 (base: romain/rc-451; do not merge its synthetic reports).

Verified so far:

  • The SARIF Ruling Diff Comment workflow passed on RC-451 Smoke-test SARIF ruling actions (do not merge) #6446 and posted a summary for both S106 and S112, including the issue messages, source snippets, and column range.
  • Both smoke-test reports pass the exact SARIF 2.1.0 schema bundled with LITS master.
  • Action/workflow YAML parsing and whitespace validation pass.

Full build/ruling pipelines are still running; baseline regeneration is intentionally delegated to the automatic ruling-update PR.

@datadog-sonarsource

datadog-sonarsource Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Pipelines

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 91df905 | Docs | Give us feedback!

Comment thread .github/workflows/build.yml Resolved
gitar-bot[bot]

This comment was marked as resolved.

@romainbrenguier

Copy link
Copy Markdown
Contributor Author

CI follow-up fixes are now pushed as separate commits:

  • Generate a unified diff for new SARIF files even when no baseline exists (--new-file), and create the expected directory before diffing.
  • Preserve expected/.gitkeep so the automatic ruling-update action can discover the ruling root after all JSON expectations are deleted.

The empty-baseline HTML report generation was reproduced locally with the same diff2html-cli 5.2.15 used by CI and now passes. The initial Linux ruling jobs successfully generated and uploaded SARIF artifacts; their test failures are the expected missing-baseline differences. The stacked smoke-test Linux artifact upload/HTML steps also passed. Updated CI runs will validate the follow-up fixes and automatic PR generation.

@gitar-bot
gitar-bot Bot dismissed their stale review November 2, 2026 10:02

✅ All code review findings resolved.

Configure merge blocking

@romainbrenguier

Copy link
Copy Markdown
Contributor Author

End-to-end SARIF smoke-test validation succeeded on #6446:

  • Ruling Diff Comment passed and rendered both synthetic reports with messages, source snippets, and precise locations.
  • Linux actual-result artifact uploads and HTML diff uploads passed.
  • Ruling Update and Notify passed and automatically created Update ruling results for PR #6446 #6447 containing regenerated SARIF baselines (targets the disposable smoke-test branch).
  • The migration branch now also preserves the empty expected root and handles first-generation diffs; refreshed CI is running for those follow-ups.

Do not merge #6446 or its automatic update PR #6447 into the migration. The migration will receive its own automatic baseline PR.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6448

1 similar comment
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6448

romainbrenguier pushed a commit that referenced this pull request Nov 3, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6465

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 1266 rule files: 0 issues removed, 164156 issues added.

S103 (java) on commons-beanutils - 0 issues removed, 24 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/BeanPropertyValueChangeClosure.java (line 134, column 1, end column 125)

Split this 124 characters long line (which is greater than 120 authorized).
       129 |      * object.
       130 |      * @param ignoreNull Determines whether <code>null</code> objects in the property path will
       131 |      * genenerate an <code>IllegalArgumentException</code> or not.
       132 |      * @throws IllegalArgumentException If the propertyName provided is null or empty.
       133 |      */
>>>    134 |     public BeanPropertyValueChangeClosure(final String propertyName, final Object propertyValue, final boolean ignoreNull) {
       135 |         super();
       136 | 
       137 |         if (propertyName != null && propertyName.length() > 0) {
       138 |             this.propertyName = propertyName;
       139 |             this.propertyValue = propertyValue;

Added src/main/java/org/apache/commons/beanutils2/BeanPropertyValueEqualsPredicate.java (line 164, column 1, end column 127)

Split this 126 characters long line (which is greater than 120 authorized).
       159 |      * @param propertyValue The value to use in object evaluation.
       160 |      * @param ignoreNull Determines whether <code>null</code> objects in the property path will
       161 |      * genenerate an <code>IllegalArgumentException</code> or not.
       162 |      * @throws IllegalArgumentException If the property name provided is null or empty.
       163 |      */
>>>    164 |     public BeanPropertyValueEqualsPredicate(final String propertyName, final Object propertyValue, final boolean ignoreNull) {
       165 |         super();
       166 | 
       167 |         if (propertyName != null && propertyName.length() > 0) {
       168 |             this.propertyName = propertyName;
       169 |             this.propertyValue = propertyValue;

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 1319, column 1, end column 123)

Split this 122 characters long line (which is greater than 120 authorized).
      1314 |          * @param cls  the class to reflect, must not be null
      1315 |          * @param methodName  the method name to obtain
      1316 |          * @param paramTypes the array of classes representing the parameter types
      1317 |          * @param exact whether the match has to be exact.
      1318 |          */
>>>   1319 |         public MethodDescriptor(final Class<?> cls, final String methodName, Class<?>[] paramTypes, final boolean exact) {
      1320 |             if (cls == null) {
      1321 |                 throw new IllegalArgumentException("Class cannot be null");
      1322 |             }
      1323 |             if (methodName == null) {
      1324 |                 throw new IllegalArgumentException("Method Name cannot be null");

Added src/main/java/org/apache/commons/beanutils2/RowSetDynaClass.java (line 236, column 1, end column 126)

Split this 125 characters long line (which is greater than 120 authorized).
       231 |      *  is <code>null</code>
       232 |      * @throws SQLException if the metadata for this result set
       233 |      *  cannot be introspected
       234 |      * @since 1.8.3
       235 |      */
>>>    236 |     public RowSetDynaClass(final ResultSet resultSet, final boolean lowerCase, final int limit, final boolean useColumnLabel)
       237 |                                                             throws SQLException {
       238 | 
       239 |         if (resultSet == null) {
       240 |             throw new NullPointerException();
       241 |         }

Added src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java (line 555, column 1, end column 128)

Split this 127 characters long line (which is greater than 120 authorized).
       550 |      * @param format The DateFormat to parse the String value.
       551 |      *
       552 |      * @return The converted Calendar object.
       553 |      * @throws ConversionException if the String cannot be converted.
       554 |      */
>>>    555 |     private Calendar parse(final Class<?> sourceType, final Class<?> targetType, final String value, final DateFormat format) {
       556 |         logFormat("Parsing", format);
       557 |         format.setLenient(false);
       558 |         final ParsePosition pos = new ParsePosition(0);
       559 |         final Date parsedDate = format.parse(value, pos); // ignore the result (use the Calendar)
       560 |         if (pos.getErrorIndex() >= 0 || pos.getIndex() != value.length() || parsedDate == null) {

Added src/main/java/org/apache/commons/beanutils2/converters/NumberConverter.java (line 546, column 1, end column 128)

Split this 127 characters long line (which is greater than 120 authorized).
       541 |      * @param format The NumberFormat to parse the String value.
       542 |      *
       543 |      * @return The converted Number object.
       544 |      * @throws ConversionException if the String cannot be converted.
       545 |      */
>>>    546 |     private Number parse(final Class<?> sourceType, final Class<?> targetType, final String value, final NumberFormat format) {
       547 |         final ParsePosition pos = new ParsePosition(0);
       548 |         final Number parsedNumber = format.parse(value, pos);
       549 |         if (pos.getErrorIndex() >= 0 || pos.getIndex() != value.length() || parsedNumber == null) {
       550 |             String msg = "Error converting from '" + toString(sourceType) + "' to '" + toString(targetType) + "'";
       551 |             if (format instanceof DecimalFormat) {

Added src/main/java/org/apache/commons/beanutils2/locale/BaseLocaleConverter.java (line 111, column 1, end column 132)

Split this 131 characters long line (which is greater than 120 authorized).
       106 |      * @param defaultValue  The default value to be returned
       107 |      * @param locale        The locale
       108 |      * @param pattern       The convertion pattern
       109 |      * @param locPattern    Indicate whether the pattern is localized or not
       110 |      */
>>>    111 |     protected BaseLocaleConverter(final Object defaultValue, final Locale locale, final String pattern, final boolean locPattern) {
       112 | 
       113 |         this(defaultValue, locale, pattern, true, locPattern);
       114 |     }
       115 | 
       116 |     /**

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtils.java (line 618, column 1, end column 141)

Split this 140 characters long line (which is greater than 120 authorized).
       613 |      * @throws InvocationTargetException if the property accessor method
       614 |      *  throws an exception
       615 |      *
       616 |      * @see LocaleBeanUtilsBean#invokeSetter(Object, String, String, int, Object)
       617 |      */
>>>    618 |     protected static void invokeSetter(final Object target, final String propName, final String key, final int index, final Object newValue)
       619 |             throws IllegalAccessException, InvocationTargetException {
       620 | 
       621 |        LocaleBeanUtilsBean.getLocaleBeanUtilsInstance().invokeSetter(target, propName, key, index, newValue);
       622 |     }
       623 | }

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtilsBean.java (line 875, column 1, end column 134)

Split this 133 characters long line (which is greater than 120 authorized).
       870 |      * @throws IllegalAccessException if the caller does not have
       871 |      *  access to the property accessor method
       872 |      * @throws InvocationTargetException if the property accessor method
       873 |      *  throws an exception
       874 |      */
>>>    875 |     protected void invokeSetter(final Object target, final String propName, final String key, final int index, final Object newValue)
       876 |             throws IllegalAccessException, InvocationTargetException {
       877 | 
       878 |         try {
       879 |             if (index >= 0) {
       880 |                 getPropertyUtils().setIndexedProperty(target, propName,

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleConvertUtils.java (line 235, column 1, end column 123)

Split this 122 characters long line (which is greater than 120 authorized).
       230 |      * @param locale The locale
       231 |      * @param pattern The convertion pattern
       232 |      * @return the converted value
       233 |      * @see LocaleConvertUtilsBean#convert(String[], Class, Locale, String)
       234 |      */
>>>    235 |     public static Object convert(final String[] values, final Class<?> clazz, final Locale locale, final String pattern) {
       236 | 
       237 |         return LocaleConvertUtilsBean.getInstance().convert(values, clazz, locale, pattern);
       238 |     }
       239 | 
       240 |     /**

Added src/main/java/org/apache/commons/beanutils2/locale/converters/BigDecimalLocaleConverter.java (line 197, column 1, end column 135)

Split this 134 characters long line (which is greater than 120 authorized).
       192 |      * @param defaultValue  The default value to be returned
       193 |      * @param locale        The locale
       194 |      * @param pattern       The convertion pattern
       195 |      * @param locPattern    Indicate whether the pattern is localized or not
       196 |      */
>>>    197 |     public BigDecimalLocaleConverter(final Object defaultValue, final Locale locale, final String pattern, final boolean locPattern) {
       198 | 
       199 |         super(defaultValue, locale, pattern, locPattern);
       200 |     }
       201 | 
       202 |     /**

Added src/main/java/org/apache/commons/beanutils2/locale/converters/BigIntegerLocaleConverter.java (line 198, column 1, end column 135)

Split this 134 characters long line (which is greater than 120 authorized).
       193 |      * @param defaultValue  The default value to be returned
       194 |      * @param locale        The locale
       195 |      * @param pattern       The convertion pattern
       196 |      * @param locPattern    Indicate whether the pattern is localized or not
       197 |      */
>>>    198 |     public BigIntegerLocaleConverter(final Object defaultValue, final Locale locale, final String pattern, final boolean locPattern) {
       199 | 
       200 |         super(defaultValue, locale, pattern, locPattern);
       201 |     }
       202 | 
       203 |     /**

Added src/main/java/org/apache/commons/beanutils2/locale/converters/ByteLocaleConverter.java (line 198, column 1, end column 129)

Split this 128 characters long line (which is greater than 120 authorized).
       193 |      * @param defaultValue  The default value to be returned
       194 |      * @param locale        The locale
       195 |      * @param pattern       The convertion pattern
       196 |      * @param locPattern    Indicate whether the pattern is localized or not
       197 |      */
>>>    198 |     public ByteLocaleConverter(final Object defaultValue, final Locale locale, final String pattern, final boolean locPattern) {
       199 | 
       200 |         super(defaultValue, locale, pattern, locPattern);
       201 |     }
       202 | 
       203 |     /**

Added src/main/java/org/apache/commons/beanutils2/locale/converters/DateLocaleConverter.java (line 219, column 1, end column 129)

Split this 128 characters long line (which is greater than 120 authorized).
       214 |      * @param defaultValue  The default value to be returned
       215 |      * @param locale        The locale
       216 |      * @param pattern       The convertion pattern
       217 |      * @param locPattern    Indicate whether the pattern is localized or not
       218 |      */
>>>    219 |     public DateLocaleConverter(final Object defaultValue, final Locale locale, final String pattern, final boolean locPattern) {
       220 | 
       221 |         super(defaultValue, locale, pattern, locPattern);
       222 |     }
       223 | 
       224 |     // --------------------------------------------------------- Methods

Added src/main/java/org/apache/commons/beanutils2/locale/converters/DecimalLocaleConverter.java (line 208, column 1, end column 132)

Split this 131 characters long line (which is greater than 120 authorized).
       203 |      * @param defaultValue  The default value to be returned
       204 |      * @param locale        The locale
       205 |      * @param pattern       The convertion pattern
       206 |      * @param locPattern    Indicate whether the pattern is localized or not
       207 |      */
>>>    208 |     public DecimalLocaleConverter(final Object defaultValue, final Locale locale, final String pattern, final boolean locPattern) {
       209 | 
       210 |         super(defaultValue, locale, pattern, locPattern);
       211 | 
       212 |     }
       213 | 
S104 (java) on commons-beanutils - 0 issues removed, 1 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java (file-level)

This file has 1,169 lines, which is greater than 750 authorized. Split it into smaller files.
>>> FILE-LEVEL ISSUE
         1 | /*
         2 |  * Licensed to the Apache Software Foundation (ASF) under one or more
         3 |  * contributor license agreements.  See the NOTICE file distributed with
         4 |  * this work for additional information regarding copyright ownership.
         5 |  * The ASF licenses this file to You under the Apache License, Version 2.0
         6 |  * (the "License"); you may not use this file except in compliance with
         7 |  * the License.  You may obtain a copy of the License at
         8 |  *
         9 |  *      http://www.apache.org/licenses/LICENSE-2.0
        10 |  *
        11 |  * Unless required by applicable law or agreed to in writing, software
S105 (java) on commons-beanutils - 0 issues removed, 2 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java (file-level)

Replace all tab characters in this file by sequences of white-spaces.
>>> FILE-LEVEL ISSUE
         1 | /*
         2 |  * Licensed to the Apache Software Foundation (ASF) under one or more
         3 |  * contributor license agreements.  See the NOTICE file distributed with
         4 |  * this work for additional information regarding copyright ownership.
         5 |  * The ASF licenses this file to You under the Apache License, Version 2.0
         6 |  * (the "License"); you may not use this file except in compliance with
         7 |  * the License.  You may obtain a copy of the License at
         8 |  *
         9 |  *      http://www.apache.org/licenses/LICENSE-2.0
        10 |  *
        11 |  * Unless required by applicable law or agreed to in writing, software

Added src/main/java/org/apache/commons/beanutils2/DynaProperty.java (file-level)

Replace all tab characters in this file by sequences of white-spaces.
>>> FILE-LEVEL ISSUE
         1 | /*
         2 |  * Licensed to the Apache Software Foundation (ASF) under one or more
         3 |  * contributor license agreements.  See the NOTICE file distributed with
         4 |  * this work for additional information regarding copyright ownership.
         5 |  * The ASF licenses this file to You under the Apache License, Version 2.0
         6 |  * (the "License"); you may not use this file except in compliance with
         7 |  * the License.  You may obtain a copy of the License at
         8 |  *
         9 |  *      http://www.apache.org/licenses/LICENSE-2.0
        10 |  *
        11 |  * Unless required by applicable law or agreed to in writing, software
S106 (java) on commons-beanutils - 0 issues removed, 2 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/BeanMap.java (line 855, column 9, end column 19)

Replace this use of System.out by a logger.
       850 |      *
       851 |      * @param ex  the exception to log
       852 |      */
       853 |     protected void logInfo(final Exception ex) {
       854 |         // Deliberately do not use LOG4J or Commons Logging to avoid dependencies
>>>    855 |         System.out.println( "INFO: Exception: " + ex );
       856 |     }
       857 | 
       858 |     /**
       859 |      * Logs the given exception to <code>System.err</code>.  Used to display
       860 |      * errors while accessing/mutating the bean.

Added src/main/java/org/apache/commons/beanutils2/BeanMap.java (line 866, column 9, end column 19)

Replace this use of System.out by a logger.
       861 |      *
       862 |      * @param ex  the exception to log
       863 |      */
       864 |     protected void logWarn(final Exception ex) {
       865 |         // Deliberately do not use LOG4J or Commons Logging to avoid dependencies
>>>    866 |         System.out.println( "WARN: Exception: " + ex );
       867 |         ex.printStackTrace();
       868 |     }
       869 | }
S1066 (java) on commons-beanutils - 0 issues removed, 1 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 1295, column 13, end column 15)

Merge this if statement with the enclosing one.
      1290 |      * @param md The method descriptor
      1291 |      * @param method The method to cache
      1292 |      */
      1293 |     private static void cacheMethod(final MethodDescriptor md, final Method method) {
      1294 |         if (CACHE_METHODS) {
>>>   1295 |             if (method != null) {
      1296 |                 cache.put(md, new WeakReference<>(method));
      1297 |             }
      1298 |         }
      1299 |     }
      1300 | 
S1067 (java) on commons-beanutils - 0 issues removed, 5 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/BasicDynaBean.java (line 436, column 13 to line 444, end column 60)

Reduce the number of conditional operators (16) used in the expression (maximum allowed 3).
       431 |      * @return <code>true</code> if the source class is assignable to the
       432 |      * destination class, otherwise <code>false</code>
       433 |      */
       434 |     protected boolean isAssignable(final Class<?> dest, final Class<?> source) {
       435 | 
>>>    436 |         if (dest.isAssignableFrom(source) ||
       437 |                 dest == Boolean.TYPE && source == Boolean.class ||
       438 |                 dest == Byte.TYPE && source == Byte.class ||
       439 |                 dest == Character.TYPE && source == Character.class ||
       440 |                 dest == Double.TYPE && source == Double.class ||
       441 |                 dest == Float.TYPE && source == Float.class ||

Added src/main/java/org/apache/commons/beanutils2/LazyDynaBean.java (line 848, column 13 to line 852, end column 46)

Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
       843 |      * @param type The class of the property
       844 |      * @return The new value
       845 |      */
       846 |     protected Object createOtherProperty(final String name, final Class<?> type) {
       847 | 
>>>    848 |         if (type == Object.class    ||
       849 |             type == String.class    ||
       850 |             type == Boolean.class   ||
       851 |             type == Character.class ||
       852 |             Date.class.isAssignableFrom(type)) {
       853 | 

Added src/main/java/org/apache/commons/beanutils2/LazyDynaBean.java (line 929, column 13 to line 937, end column 60)

Reduce the number of conditional operators (16) used in the expression (maximum allowed 3).
       924 |      * @return <code>true</code> if the source class is assignable to the
       925 |      * destination class, otherwise <code>false</code>
       926 |      */
       927 |     protected boolean isAssignable(final Class<?> dest, final Class<?> source) {
       928 | 
>>>    929 |         if (dest.isAssignableFrom(source) ||
       930 |                 dest == Boolean.TYPE && source == Boolean.class ||
       931 |                 dest == Byte.TYPE && source == Byte.class ||
       932 |                 dest == Character.TYPE && source == Character.class ||
       933 |                 dest == Double.TYPE && source == Double.class ||
       934 |                 dest == Float.TYPE && source == Float.class ||

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 1067, column 25 to line 1071, end column 59)

Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
      1062 |             final Log log = LogFactory.getLog(MethodUtils.class);
      1063 |             if (!loggedAccessibleWarning) {
      1064 |                 boolean vulnerableJVM = false;
      1065 |                 try {
      1066 |                     final String specVersion = System.getProperty("java.specification.version");
>>>   1067 |                     if (specVersion.charAt(0) == '1' &&
      1068 |                             (specVersion.charAt(2) == '0' ||
      1069 |                              specVersion.charAt(2) == '1' ||
      1070 |                              specVersion.charAt(2) == '2' ||
      1071 |                              specVersion.charAt(2) == '3')) {
      1072 | 

Added src/main/java/org/apache/commons/beanutils2/locale/converters/StringLocaleConverter.java (line 236, column 13 to line 240, end column 39)

Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
       231 |     @Override
       232 |     protected Object parse(final Object value, final String pattern) throws ParseException {
       233 | 
       234 |         String result = null;
       235 | 
>>>    236 |         if (value instanceof Integer ||
       237 |                 value instanceof Long ||
       238 |                 value instanceof BigInteger ||
       239 |                 value instanceof Byte ||
       240 |                 value instanceof Short) {
       241 | 
S108 (java) on commons-beanutils - 0 issues removed, 3 issues added - new ruling file

Added src/test/java/org/apache/commons/beanutils2/MappedPropertyTestCase.java (line 290, column 38, end column 39)

Remove this block of code, fill it in, or add a comment explaining why it is empty.
       285 |         final String property = "protectedMapped";
       286 |         final Class<?> clazz = MappedPropertyChildBean.class;
       287 |         try {
       288 |             new MappedPropertyDescriptor(property, clazz);
       289 |             fail("Property '" + property + "' found in " + clazz.getName());
>>>    290 |         } catch (final Exception ex) {
       291 |         }
       292 |     }
       293 | 
       294 | 
       295 |     /**

Added src/test/java/org/apache/commons/beanutils2/MappedPropertyTestCase.java (line 320, column 38, end column 39)

Remove this block of code, fill it in, or add a comment explaining why it is empty.
       315 |         final String property = "XXXXXX";
       316 |         final Class<?> clazz = MappedPropertyTestInterface.class;
       317 |         try {
       318 |             new MappedPropertyDescriptor(property, clazz);
       319 |             fail("Property '" + property + "' found in " + clazz.getName());
>>>    320 |         } catch (final Exception ex) {
       321 |         }
       322 |     }
       323 | 
       324 |     /**
       325 |      * Test Interface Inherited mapped property

Added src/test/java/org/apache/commons/beanutils2/bugs/Jira347TestCase.java (line 114, column 47, end column 48)

Remove this block of code, fill it in, or add a comment explaining why it is empty.
       109 |             try {
       110 |                 long i = 0;
       111 |                 while (true && ref.get() != null) {
       112 |                     list.add("A Big String A Big String A Big String A Big String A Big String A Big String A Big String A Big String A Big String A Big String " + i++);
       113 |                 }
>>>    114 |             } catch (final Throwable ignored) {
       115 |             }
       116 |             list.clear();
       117 |             list = null;
       118 |             // System.out.println("Count " + count + " : " + getMemoryStats());
       119 |             System.gc();
S109 (java) on commons-beanutils - 0 issues removed, 19 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java (line 119, column 68, end column 69)

Assign this magic number 2 to a well-named constant, and use the constant instead.
       114 |              */
       115 |         }
       116 | 
       117 |         // If there's no read method, then look for just a write method
       118 |         if (mappedReadMethod == null) {
>>>    119 |             mappedWriteMethod = getMethod(beanClass, "set" + base, 2);
       120 |         }
       121 | 
       122 |         if (mappedReadMethod == null && mappedWriteMethod == null) {
       123 |             throw new IntrospectionException("Property '" + propertyName +
       124 |                     "' not found on " +

Added src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java (line 175, column 56, end column 57)

Assign this magic number 2 to a well-named constant, and use the constant instead.
       170 |             final Class<?>[] params = { String.class, mappedReadMethod.getReturnType() };
       171 |             mappedWriteMethod =
       172 |                 getMethod(beanClass, mappedSetterName, params);
       173 |         } else {
       174 |             mappedWriteMethod =
>>>    175 |                 getMethod(beanClass, mappedSetterName, 2);
       176 |         }
       177 |         mappedReadMethodRef  = new MappedMethodReference(mappedReadMethod);
       178 |         mappedWriteMethodRef = new MappedMethodReference(mappedWriteMethod);
       179 | 
       180 |         findMappedPropertyType();

Added src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java (line 302, column 38, end column 39)

Assign this magic number 2 to a well-named constant, and use the constant instead.
       297 |                 }
       298 |             }
       299 | 
       300 |             if (mappedWriteMethod != null) {
       301 |                 final Class<?>[] params = mappedWriteMethod.getParameterTypes();
>>>    302 |                 if (params.length != 2) {
       303 |                     throw new IntrospectionException
       304 |                             ("bad mapped write method arg count");
       305 |                 }
       306 |                 if (mappedPropertyType != null &&
       307 |                         mappedPropertyType != params[1]) {

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 1068, column 49, end column 50)

Assign this magic number 2 to a well-named constant, and use the constant instead.
      1063 |             if (!loggedAccessibleWarning) {
      1064 |                 boolean vulnerableJVM = false;
      1065 |                 try {
      1066 |                     final String specVersion = System.getProperty("java.specification.version");
      1067 |                     if (specVersion.charAt(0) == '1' &&
>>>   1068 |                             (specVersion.charAt(2) == '0' ||
      1069 |                              specVersion.charAt(2) == '1' ||
      1070 |                              specVersion.charAt(2) == '2' ||
      1071 |                              specVersion.charAt(2) == '3')) {
      1072 | 
      1073 |                         vulnerableJVM = true;

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 1069, column 49, end column 50)

Assign this magic number 2 to a well-named constant, and use the constant instead.
      1064 |                 boolean vulnerableJVM = false;
      1065 |                 try {
      1066 |                     final String specVersion = System.getProperty("java.specification.version");
      1067 |                     if (specVersion.charAt(0) == '1' &&
      1068 |                             (specVersion.charAt(2) == '0' ||
>>>   1069 |                              specVersion.charAt(2) == '1' ||
      1070 |                              specVersion.charAt(2) == '2' ||
      1071 |                              specVersion.charAt(2) == '3')) {
      1072 | 
      1073 |                         vulnerableJVM = true;
      1074 |                     }

Added src/main/java/org/apache/commons/beanutils2/MethodUtils.java (line 1070, column 49, end column 50)

Assign this magic number 2 to a well-named constant, and use the constant instead.
      1065 |                 try {
      1066 |                     final String specVersion = System.getProperty("java.specification.version");
      1067 |                     if (specVersion.charAt(0) == '1' &&
      1068 |                             (specVersion.charAt(2) == '0' ||
      1069 |                              specVersion.charAt(2) == '1' ||
>>>   1070 |                              specVersion.charAt(2) == '2' ||
      1071 |                              specVersion.charAt(2) == '3')) {
      1072 | 
      1073 |                         vulnerableJVM = true;
      1074 |                     }
      1075 |                 } catch (final SecurityException e) {

Added src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java (line 1619, column 55, end column 56)

Assign this magic number 2 to a well-named constant, and use the constant instead.
      1614 |         if (descriptor instanceof IndexedPropertyDescriptor) {
      1615 |             Method writeMethod = ((IndexedPropertyDescriptor) descriptor).
      1616 |                     getIndexedWriteMethod();
      1617 |             writeMethod = MethodUtils.getAccessibleMethod(bean.getClass(), writeMethod);
      1618 |             if (writeMethod != null) {
>>>   1619 |                 final Object[] subscript = new Object[2];
      1620 |                 subscript[0] = Integer.valueOf(index);
      1621 |                 subscript[1] = value;
      1622 |                 try {
      1623 |                     if (log.isTraceEnabled()) {
      1624 |                         final String valueClassName =

Added src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java (line 1787, column 52, end column 53)

Assign this magic number 2 to a well-named constant, and use the constant instead.
      1782 |             Method mappedWriteMethod =
      1783 |                     ((MappedPropertyDescriptor) descriptor).
      1784 |                     getMappedWriteMethod();
      1785 |             mappedWriteMethod = MethodUtils.getAccessibleMethod(bean.getClass(), mappedWriteMethod);
      1786 |             if (mappedWriteMethod != null) {
>>>   1787 |                 final Object[] params = new Object[2];
      1788 |                 params[0] = key;
      1789 |                 params[1] = value;
      1790 |                 if (log.isTraceEnabled()) {
      1791 |                     final String valueClassName =
      1792 |                         value == null ? "<null>" : value.getClass().getName();

Added src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java (line 303, column 57, end column 61)

Assign this magic number 1000 to a well-named constant, and use the constant instead.
       298 |             // ---------------------- JDK 1.3 Fix ----------------------
       299 |             // N.B. Prior to JDK 1.4 the Timestamp's getTime() method
       300 |             //      didn't include the milliseconds. The following code
       301 |             //      ensures it works consistently accross JDK versions
       302 |             final java.sql.Timestamp timestamp = (java.sql.Timestamp)value;
>>>    303 |             long timeInMillis = ((timestamp.getTime() / 1000) * 1000);
       304 |             timeInMillis += timestamp.getNanos() / 1000000;
       305 |             // ---------------------- JDK 1.3 Fix ----------------------
       306 |             return toDate(targetType, timeInMillis);
       307 |         }
       308 | 

Added src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java (line 303, column 65, end column 69)

Assign this magic number 1000 to a well-named constant, and use the constant instead.
       298 |             // ---------------------- JDK 1.3 Fix ----------------------
       299 |             // N.B. Prior to JDK 1.4 the Timestamp's getTime() method
       300 |             //      didn't include the milliseconds. The following code
       301 |             //      ensures it works consistently accross JDK versions
       302 |             final java.sql.Timestamp timestamp = (java.sql.Timestamp)value;
>>>    303 |             long timeInMillis = ((timestamp.getTime() / 1000) * 1000);
       304 |             timeInMillis += timestamp.getNanos() / 1000000;
       305 |             // ---------------------- JDK 1.3 Fix ----------------------
       306 |             return toDate(targetType, timeInMillis);
       307 |         }
       308 | 

Added src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java (line 304, column 52, end column 59)

Assign this magic number 1000000 to a well-named constant, and use the constant instead.
       299 |             // N.B. Prior to JDK 1.4 the Timestamp's getTime() method
       300 |             //      didn't include the milliseconds. The following code
       301 |             //      ensures it works consistently accross JDK versions
       302 |             final java.sql.Timestamp timestamp = (java.sql.Timestamp)value;
       303 |             long timeInMillis = ((timestamp.getTime() / 1000) * 1000);
>>>    304 |             timeInMillis += timestamp.getNanos() / 1000000;
       305 |             // ---------------------- JDK 1.3 Fix ----------------------
       306 |             return toDate(targetType, timeInMillis);
       307 |         }
       308 | 
       309 |         // Handle Date (includes java.sql.Date & java.sql.Time)

Added src/main/java/org/apache/commons/beanutils2/expression/DefaultResolver.java (line 98, column 53, end column 55)

Assign this magic number 10 to a well-named constant, and use the constant instead.
        93 |                 if (value.length() == 0) {
        94 |                     throw new IllegalArgumentException("No Index Value");
        95 |                 }
        96 |                 int index = 0;
        97 |                 try {
>>>     98 |                     index = Integer.parseInt(value, 10);
        99 |                 } catch (final Exception e) {
       100 |                     throw new IllegalArgumentException("Invalid index value '"
       101 |                             + value + "'");
       102 |                 }
       103 |                 return index;
S110 (java) on commons-beanutils - 0 issues removed, 1 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/NestedNullException.java (line 27, column 14, end column 33)

This class has 6 parents which is greater than 5 authorized.
        22 |  * against given bean since a nested bean referenced is null.
        23 |  *
        24 |  * @since 1.7
        25 |  */
        26 | 
>>>     27 | public class NestedNullException extends BeanAccessLanguageException {
        28 | 
        29 |     private static final long serialVersionUID = 1L;
        30 | 
        31 |     // --------------------------------------------------------- Constuctors
        32 | 
S1105 (java) on commons-beanutils - 0 issues removed, 2 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/BeanComparator.java (line 201, column 9, end column 10)

Move this left curly brace to the end of previous line of code.
       196 | 
       197 |         if (!comparator.equals(beanComparator.comparator)) {
       198 |             return false;
       199 |         }
       200 |         if (property != null)
>>>    201 |         {
       202 |             if (!property.equals(beanComparator.property)) {
       203 |                 return false;
       204 |             }
       205 |         }
       206 |         else

Added src/main/java/org/apache/commons/beanutils2/BeanComparator.java (line 207, column 9, end column 10)

Move this left curly brace to the end of previous line of code.
       202 |             if (!property.equals(beanComparator.property)) {
       203 |                 return false;
       204 |             }
       205 |         }
       206 |         else
>>>    207 |         {
       208 |             return beanComparator.property == null;
       209 |         }
       210 | 
       211 |         return true;
       212 |     }
S1107 (java) on commons-beanutils - 0 issues removed, 53 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/BeanComparator.java (line 168, column 9, end column 14)

Move this "catch" on the same line that the previous closing curly brace.
       163 |         try {
       164 |             final Object value1 = PropertyUtils.getProperty( o1, property );
       165 |             final Object value2 = PropertyUtils.getProperty( o2, property );
       166 |             return internalCompare( value1, value2 );
       167 |         }
>>>    168 |         catch ( final IllegalAccessException iae ) {
       169 |             throw new RuntimeException( "IllegalAccessException: " + iae.toString() );
       170 |         }
       171 |         catch ( final InvocationTargetException ite ) {
       172 |             throw new RuntimeException( "InvocationTargetException: " + ite.toString() );
       173 |         }

Added src/main/java/org/apache/commons/beanutils2/BeanComparator.java (line 171, column 9, end column 14)

Move this "catch" on the same line that the previous closing curly brace.
       166 |             return internalCompare( value1, value2 );
       167 |         }
       168 |         catch ( final IllegalAccessException iae ) {
       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 |         }

Added src/main/java/org/apache/commons/beanutils2/BeanComparator.java (line 174, column 9, end column 14)

Move this "catch" on the same line that the previous closing curly brace.
       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 295, column 9, end column 14)

Move this "catch" on the same line that the previous closing curly brace.
       290 |         Class<? extends Object> beanClass = null;
       291 |         try {
       292 |             beanClass = bean.getClass();
       293 |             bean = beanClass.newInstance();
       294 |         }
>>>    295 |         catch (final Exception e) {
       296 |             final UnsupportedOperationException uoe =
       297 |                 new UnsupportedOperationException("Could not create new instance of class: " + beanClass);
       298 |             BeanUtils.initCause(uoe, e);
       299 |             throw uoe;
       300 |         }

Added src/main/java/org/apache/commons/beanutils2/BeanMap.java (line 361, column 17, end column 22)

Move this "catch" on the same line that the previous closing curly brace.
       356 |             final Method method = getReadMethod( name );
       357 |             if ( method != null ) {
       358 |                 try {
       359 |                     return method.invoke( bean, NULL_ARGUMENTS );
       360 |                 }
>>>    361 |                 catch (  final IllegalAccessException e ) {
       362 |                     logWarn( e );
       363 |                 }
       364 |                 catch ( final IllegalArgumentException e ) {
       365 |                     logWarn(  e );
       366 |                 }

Added src/main/java/org/apache/commons/beanutils2/BeanMap.java (line 364, column 17, end column 22)

Move this "catch" on the same line that the previous closing curly brace.
       359 |                     return method.invoke( bean, NULL_ARGUMENTS );
       360 |                 }
       361 |                 catch (  final IllegalAccessException e ) {
       362 |                     logWarn( e );
       363 |                 }
>>>    364 |                 catch ( final IllegalArgumentException e ) {
       365 |                     logWarn(  e );
       366 |                 }
       367 |                 catch ( final InvocationTargetException e ) {
       368 |                     logWarn(  e );
       369 |                 }

Added src/main/java/org/apache/commons/beanutils2/LazyDynaBean.java (line 725, column 17, end column 22)

Move this "catch" on the same line that the previous closing curly brace.
       720 |                 indexedProperty = defaultIndexedProperty(name);
       721 |             } else {
       722 |                 try {
       723 |                     indexedProperty = type.newInstance();
       724 |                 }
>>>    725 |                 catch (final Exception ex) {
       726 |                     throw new IllegalArgumentException
       727 |                         ("Error instantiating indexed property of type '" +
       728 |                                    type.getName() + "' for '" + name + "' " + ex);
       729 |                 }
       730 |             }

Added src/main/java/org/apache/commons/beanutils2/LazyDynaBean.java (line 764, column 13, end column 18)

Move this "catch" on the same line that the previous closing curly brace.
       759 | 
       760 |         } else if (Map.class.isAssignableFrom(type)) {
       761 |             try {
       762 |                 mappedProperty = type.newInstance();
       763 |             }
>>>    764 |             catch (final Exception ex) {
       765 |                 throw new IllegalArgumentException
       766 |                     ("Error instantiating mapped property of type '" +
       767 |                             type.getName() + "' for '" + name + "' " + ex);
       768 |             }
       769 |         } else {

Added src/main/java/org/apache/commons/beanutils2/LazyDynaBean.java (line 789, column 9, end column 14)

Move this "catch" on the same line that the previous closing curly brace.
       784 |      */
       785 |     protected Object createDynaBeanProperty(final String name, final Class<?> type) {
       786 |         try {
       787 |             return type.newInstance();
       788 |         }
>>>    789 |         catch (final Exception ex) {
       790 |             if (logger().isWarnEnabled()) {
       791 |                 logger().warn("Error instantiating DynaBean property of type '" +
       792 |                         type.getName() + "' for '" + name + "' " + ex);
       793 |             }
       794 |             return null;

Added src/main/java/org/apache/commons/beanutils2/ResultSetDynaClass.java (line 243, column 9, end column 14)

Move this "catch" on the same line that the previous closing curly brace.
       238 |     protected Class<?> loadClass(final String className) throws SQLException {
       239 | 
       240 |         try {
       241 |             return getClass().getClassLoader().loadClass(className);
       242 |         }
>>>    243 |         catch (final Exception e) {
       244 |             throw new SQLException("Cannot load column class '" +
       245 |                                    className + "': " + e);
       246 |         }
       247 |     }
       248 | }

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtilsBean.java (line 647, column 13, end column 17)

Move this "else" on the same line that the previous closing curly brace.
       642 |             sb.append(name);
       643 |             sb.append(", ");
       644 |             if (value == null) {
       645 |                 sb.append("<NULL>");
       646 |             }
>>>    647 |             else if (value instanceof String) {
       648 |                 sb.append((String) value);
       649 |             }
       650 |             else if (value instanceof String[]) {
       651 |                 final String[] values = (String[]) value;
       652 |                 sb.append('[');

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtilsBean.java (line 650, column 13, end column 17)

Move this "else" on the same line that the previous closing curly brace.
       645 |                 sb.append("<NULL>");
       646 |             }
       647 |             else if (value instanceof String) {
       648 |                 sb.append((String) value);
       649 |             }
>>>    650 |             else if (value instanceof String[]) {
       651 |                 final String[] values = (String[]) value;
       652 |                 sb.append('[');
       653 |                 for (int i = 0; i < values.length; i++) {
       654 |                     if (i > 0) {
       655 |                         sb.append(',');

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtilsBean.java (line 661, column 13, end column 17)

Move this "else" on the same line that the previous closing curly brace.
       656 |                     }
       657 |                     sb.append(values[i]);
       658 |                 }
       659 |                 sb.append(']');
       660 |             }
>>>    661 |             else {
       662 |                 sb.append(value.toString());
       663 |             }
       664 |             sb.append(')');
       665 |             log.trace(sb.toString());
       666 |         }

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleConvertUtilsBean.java (line 144, column 9, end column 13)

Move this "else" on the same line that the previous closing curly brace.
       139 |     public void setDefaultLocale(final Locale locale) {
       140 | 
       141 |         if (locale == null) {
       142 |             defaultLocale = Locale.getDefault();
       143 |         }
>>>    144 |         else {
       145 |             defaultLocale = locale;
       146 |         }
       147 |     }
       148 | 
       149 |     /**

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleConvertUtilsBean.java (line 440, column 9, end column 13)

Move this "else" on the same line that the previous closing curly brace.
       435 |         Map<Class<?>, LocaleConverter> localeConverters;
       436 | 
       437 |         if (locale == null) {
       438 |             localeConverters = (Map<Class<?>, LocaleConverter>) mapConverters.get(defaultLocale);
       439 |         }
>>>    440 |         else {
       441 |             localeConverters = (Map<Class<?>, LocaleConverter>) mapConverters.get(locale);
       442 | 
       443 |             if (localeConverters == null) {
       444 |                 localeConverters = create(locale);
       445 |                 mapConverters.put(locale, localeConverters);

... and 1255 more rules with changes (diff too large to display fully)

romainbrenguier pushed a commit that referenced this pull request Nov 3, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6469

romainbrenguier pushed a commit that referenced this pull request Nov 3, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6471

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6471

romainbrenguier pushed a commit that referenced this pull request Nov 4, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6476

romainbrenguier pushed a commit that referenced this pull request Nov 4, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6477

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6483

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

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6484

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

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6486

github-actions Bot and others added 2 commits November 4, 2026 20:03
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@romainbrenguier
romainbrenguier marked this pull request as ready for review November 4, 2026 20:30

@BartLucien BartLucien left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some docs needs to be updated.

  • README
  • new rule skill
    Otherwise it looks good to me.
    I do not approve to make sure that someone from the owner squad sees and accepts the changes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this action still needed? I think it serves a similar purpose of what we are adding in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I created a ticket to check whether we can get rid of it https://sonarsource.atlassian.net/browse/RC-806

Comment thread .agents/skills/new-rule/SKILL.md Resolved
Co-authored-by: Romain Brenguier <11854194+romainbrenguier@users.noreply.github.com>
@gitar-bot

gitar-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 6 closed / 6 findings

🟡 Medium risk · Migrating ruling baselines to SARIF and changing upload/diff workflows could omit or misrender findings in automated ruling comparisons.

Migrates Java ruling expectations from JSON to SARIF, upgrades LITS to 0.13.0.16223, and updates the ruling diff workflow to trigger on SARIF changes. Resolved the empty expected directory breaking auto-update, stale JSON-based ruling docs, missing SARIF baselines for three projects, and an empty step 6 diff. The --new-file report of the expected/.gitkeep file as deleted is closed without a fix. No open issues remain.

✅ 6 closed
✅ Bug: Deleting every baseline removes expected/ dir, breaking auto-update PR

📄 .github/workflows/build.yml:193-196 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:166-175 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:185
This PR deletes every file under its/ruling/src/test/resources, and git does not keep empty directories, so its/ruling/src/test/resources/expected is gone from the checkout. The pinned ruling-update-and-notify@ec8eabc action first runs find_ruling_directory.py. That script only accepts a layout when <ruling_root>/expected already exists as a directory; otherwise it raises ValueError and exits 1 (find_ruling_directory.py). The sync and fix-PR steps come after it (action.yml), so the automatic ruling-update PR that the description relies on to regenerate SARIF baselines never gets created. sync_ruling_artifacts.py would create the directory, but it never runs.

The missing directory also breaks subset runs. When -DsonarRules=... is set, prepareDumpOldFolder() calls Files.list(Paths.get("src/test/resources/expected")), which throws NoSuchFileException in @BeforeClass, so every test fails before any analysis runs. The fix is to keep the directory tracked, e.g. commit its/ruling/src/test/resources/expected/.gitkeep, or commit the regenerated SARIF baselines in this PR.

✅ Quality: Ruling docs still describe JSON expectations after the SARIF switch

📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:185
This PR removes all JSON ruling baselines and makes JavaRulingTest.copyDumpSubset select java-<RULE>.sarif. The contributor and agent docs were not updated to match. AGENTS.md:301-303 still says expected and actual files are java-<RULE_ID>.json in the {"<projectKey>:path": [lines]} format. .agents/skills/new-rule/SKILL.md:88 tells the new-rule agent to read new findings from expected/<project>/java-<RULE_ID>.json with that same entry shape. README.md:167,173 describes json files that list lines per file. Those files no longer exist, so anyone following these steps (agents included) will look for files that aren't there, or parse the SARIF output the wrong way. Update these sections to the .sarif file names and the SARIF 2.1.0 runs[].results[] structure.

✅ Cross-PR: Pinning to pre-SARIF tooling would break the SARIF ruling flow

📄 .github/workflows/ruling-diff-comment.yml:7 📄 .github/workflows/ruling-diff-comment.yml:48-55 📄 .github/actions/upload-actual/action.yml:25
Breaks when #6457 merges after this PR and the conflicting uses: lines are resolved to its pinned SHA. The next ruling run then sends SARIF expectations to the old tooling actions, which were built for JSON.

How

  1. This PR points ruling-diff-comment, prepare-ruling-artifact and ruling-update-and-notify at @master and relies on their SARIF handling (.github/workflows/ruling-diff-comment.yml:48, .github/actions/upload-actual/action.yml:25, .github/workflows/build.yml:194).
  2. This PR also removes the ruling-root input and triggers only on expected/**/*.sarif (.github/workflows/ruling-diff-comment.yml:7,51-55).
  3. RC-354 Pin core-languages-tooling-public actions to the current master commit #6457 pins every one of these actions to a04d15c9. Its own description calls that revision a protective pin taken before RC-354: Expectation update tool improvements core-languages-tooling-public#85 made artifact uploads SARIF-only. Its ruling-diff-comment.yml:50 still passes ruling-root and triggers on **/*.json.
  4. With the pin in place, the SARIF artifacts, the diff comment and the automatic SARIF regeneration PR all run against the pre-SARIF actions, without ruling-root. This PR deleted every expectation and counts on that regeneration to rebuild them.

Fix

Compared fcbca51 with #6457 at 090585d

✅ Bug: SARIF baselines missing for 3 analyzed projects; ruling still fails

📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:270 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:302 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:333 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:372 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:388 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:477 📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:488-489 📄 its/ruling/src/test/resources/expected/eclipse-jetty/java-S103.sarif 📄 its/ruling/src/test/resources/expected/java-time-example/java-S109.sarif
I compared the regenerated expected/ tree with the JSON tree that existed before this PR (7c2629a~1). Every project directory came back except three: eclipse-jetty-similar-to-main (222 files before), eclipse-jetty-similar-to-main-small (6) and java-time-example-less-threshold (9). That's why the tree now has 1031 SARIF files instead of the 1274 JSON ones. The other gaps (guava S2198/S9396/S9397, sonarqube-6.5 S2245/S9133/S9396) were empty {} JSON files, so it's expected that they have no SARIF.

The cause is in JavaRulingTest. executeBuildWithCommonProperties calls assertNoDifferences(projectName) right after each build (lines 488-489). In eclipse_jetty_incremental (line 270) and java_time_example_incremental (line 372), the main-branch analysis therefore failed against the empty baseline and aborted the test method. The PR-branch analyses (lines 302, 333, 388) never ran, so LITS never wrote target/actual/<pr project> for them, and the auto-update PR had nothing to commit.

With this commit, the main-branch analyses now pass. The PR-branch analyses then run with sonar.lits.dump.old pointing at missing expected/<pr project> folders (line 477), every issue is reported as a difference, and ruling QA keeps failing. It would take at least one or two more update rounds to fill these folders in.

Fix: regenerate the three missing baselines before merging, either by running the incremental tests locally or by triggering another ruling update. Confirm that the expected/ tree covers every projectName passed to executeBuildWithCommonProperties.

✅ Bug: Step 6 diffs actual vs expected after ruling is green, so it's empty

📄 .agents/skills/new-rule/SKILL.md:86-88 📄 .agents/skills/new-rule/SKILL.md:83 📄 .agents/skills/new-rule/SKILL.md:88
Step 6 only runs "when Step 5 is fully green". By then the automatic ruling-update PR (line 83) has been merged, so expected/<project>/java-<RULE_ID>.sarif matches what the analysis produces. Ruling also runs on CI, so a local its/ruling/target/actual/ may not exist at all. The new instruction compares target/actual/...sarif with expected/...sarif, which gives zero new findings or a missing file. The TP/FP gate (TP% > 90%) then gets nothing to sample, so it either passes trivially or can't be computed. The old wording pointed at the diff of the committed expectation file, which does hold the new findings. Fix: say that new findings are the runs[].results[] entries added to expected/<project>/java-<RULE_ID>.sarif by the ruling-update commit/PR (e.g. git diff master -- its/ruling/src/test/resources/expected/).

...and 1 more closed from earlier reviews

Verification Rules 2/2 met PRs 3 inspected

Functional validation No results

Rules 2 passed 1 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Broader impact PRs 3 inspected
Cross-PR 3 PRs inspected 0 open impacts

PR 85: RC-354: Expectation update tool improvements Inspected
PR 6457: RC-354 Pin core-languages-tooling-public actions to the current master commit Inspected
PR 6418: RC-353 Test Sarif diff Inspected

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

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

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

sonarqube-next Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

This branch has not been deployed

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

3 participants