Skip to content

RC-451 Smoke-test SARIF ruling actions (do not merge) - #6446

Draft
romainbrenguier wants to merge 12 commits into
romain/rc-451from
romain/rc-451-sarif-smoke-test
Draft

romainbrenguier wants to merge 12 commits into
romain/rc-451from
romain/rc-451-sarif-smoke-test

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Stacked on #6445 (base: romain/rc-451). This is a disposable draft PR to exercise the SARIF actions introduced by the migration. Do not merge these synthetic expectations.

Changes

  • Add two small SARIF 2.1.0 reports under the standard expected//java-.sarif paths.
  • Use existing commons-beanutils S106 and guava S112 source locations.
  • Exercise both line-only and column-aware primary locations, plus SARIF issue messages.
  • Keep all synthetic data separate from the migration PR; actual baselines will be regenerated by the automatic ruling-update PR.

Expected CI behavior

  • Ruling Diff Comment triggers for these SARIF files and posts a summary for S106 and S112.
  • The Build workflow exercises LITS SARIF generation and prepare-ruling-artifact/ruling-update-and-notify. Ruling differences are intentional until automatic baseline regeneration.

Local validation

  • Workflow/action YAML parsing, report JSON checks, and git diff --check pass.
  • DEEP Vortex analysis: no issues in the SARIF reports or on other changed lines; only the three pre-existing java:S3457 findings on untouched JavaRulingTest.java lines 558–560 remain.

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 "Rebase the branch (not merge) on https://github.com/SonarSource/sonar-java/pull/6445. Keep only the only relevant commit (Exercise...)" -a "None" -g "codex"


Summary by Gitar

  • Additional ruling expectation:
    • Added java:S9438 finding for BeanMap.java line 42 to commons-beanutils expected SARIF output

This will update automatically on new commits.

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 5 rule files: 7 issues removed, 24 issues added.

S106 (java) on commons-beanutils - 0 issues removed, 1 issues added

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 | }
S9438 (java) on commons-beanutils - 1 issues removed, 0 issues added

Removed src/main/java/org/apache/commons/beanutils2/BeanMap.java (line 42, column 5, end column 7)

Make this private field "final", it is never reassigned.
        37 | import org.apache.commons.collections4.keyvalue.AbstractMapEntry;
        38 | 
        39 | /**
        40 |  * An implementation of Map for JavaBeans which uses introspection to
        41 |  * get and put properties in the bean.
>>>     42 |  * <p>
        43 |  * If an exception occurs during attempts to get or set a property then the
        44 |  * property is considered non existent in the Map
        45 |  *
        46 |  */
        47 | public class BeanMap extends AbstractMap<Object, Object> implements Cloneable {
S3626 (java) on eclipse-jetty - 3 issues removed, 3 issues added

Removed jetty-server/src/main/java/org/eclipse/jetty/server/resource/SeekableByteChannelRangeWriter.java (line 103, column 21, end column 28)

Remove this redundant return.
        98 |             {
        99 |                 if (channel.position() != skipTo)
       100 |                 {
       101 |                     channel.position(skipTo);
       102 |                     pos = skipTo;
>>>    103 |                     return;
       104 |                 }
       105 |             }
       106 |             catch (UnsupportedOperationException e)
       107 |             {
       108 |                 defaultSeekMode = false;

Added jetty-server/src/main/java/org/eclipse/jetty/server/resource/SeekableByteChannelRangeWriter.java (line 103, column 21, end column 28)

Remove this redundant jump.
        98 |             {
        99 |                 if (channel.position() != skipTo)
       100 |                 {
       101 |                     channel.position(skipTo);
       102 |                     pos = skipTo;
>>>    103 |                     return;
       104 |                 }
       105 |             }
       106 |             catch (UnsupportedOperationException e)
       107 |             {
       108 |                 defaultSeekMode = false;

Removed jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java (line 981, column 25, end column 34)

Remove this redundant continue.
       976 | 
       977 |                 case POST_DATA:
       978 |                     if (',' == ch)
       979 |                     {
       980 |                         state = CsvSplitState.PRE_DATA;
>>>    981 |                         continue;
       982 |                     }
       983 |                     continue;
       984 | 
       985 |                 default:
       986 |                     throw new IllegalStateException(state.toString());

Added jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java (line 981, column 25, end column 34)

Remove this redundant jump.
       976 | 
       977 |                 case POST_DATA:
       978 |                     if (',' == ch)
       979 |                     {
       980 |                         state = CsvSplitState.PRE_DATA;
>>>    981 |                         continue;
       982 |                     }
       983 |                     continue;
       984 | 
       985 |                 default:
       986 |                     throw new IllegalStateException(state.toString());

Removed jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java (line 88, column 13, end column 20)

Remove this redundant return.
        83 |             return;
        84 | 
        85 |         if (_capacity == 0 || !isRunning())
        86 |         {
        87 |             end(object);
>>>     88 |             return;
        89 |         }
        90 |         else if (_capacity < 0)
        91 |         {
        92 |             reset(object);
        93 |             _pool.add(object);

Added jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java (line 88, column 13, end column 20)

Remove this redundant jump.
        83 |             return;
        84 | 
        85 |         if (_capacity == 0 || !isRunning())
        86 |         {
        87 |             end(object);
>>>     88 |             return;
        89 |         }
        90 |         else if (_capacity < 0)
        91 |         {
        92 |             reset(object);
        93 |             _pool.add(object);
S112 (java) on guava - 1 issues removed, 18 issues added

Removed src/com/google/common/cache/CacheLoader.java (line 69, column 5, end column 10)

RC-451 smoke test: define and throw a dedicated exception instead of using a generic one.
        64 |    * @throws Exception if unable to load the result
        65 |    * @throws InterruptedException if this method is interrupted. {@code InterruptedException} is
        66 |    *     treated like any other {@code Exception} in all respects except that, when it is caught,
        67 |    *     the thread's interrupt status is set
        68 |    */
>>>     69 |   public abstract V load(K key) throws Exception;
        70 | 
        71 |   /**
        72 |    * Computes or retrieves a replacement value corresponding to an already-cached {@code key}. This
        73 |    * method is called when an existing cache entry is refreshed by
        74 |    * {@link CacheBuilder#refreshAfterWrite}, or through a call to {@link LoadingCache#refresh}.

Added src/com/google/common/cache/CacheLoader.java (line 69, column 40, end column 49)

Replace generic exceptions with specific library exceptions or a custom exception.
        64 |    * @throws Exception if unable to load the result
        65 |    * @throws InterruptedException if this method is interrupted. {@code InterruptedException} is
        66 |    *     treated like any other {@code Exception} in all respects except that, when it is caught,
        67 |    *     the thread's interrupt status is set
        68 |    */
>>>     69 |   public abstract V load(K key) throws Exception;
        70 | 
        71 |   /**
        72 |    * Computes or retrieves a replacement value corresponding to an already-cached {@code key}. This
        73 |    * method is called when an existing cache entry is refreshed by
        74 |    * {@link CacheBuilder#refreshAfterWrite}, or through a call to {@link LoadingCache#refresh}.

Added src/com/google/common/cache/CacheLoader.java (line 121, column 63, end column 72)

Replace generic exceptions with specific library exceptions or a custom exception.
       116 |    * @throws InterruptedException if this method is interrupted. {@code InterruptedException} is
       117 |    *     treated like any other {@code Exception} in all respects except that, when it is caught,
       118 |    *     the thread's interrupt status is set
       119 |    * @since 11.0
       120 |    */
>>>    121 |   public Map<K, V> loadAll(Iterable<? extends K> keys) throws Exception {
       122 |     // This will be caught by getAll(), causing it to fall back to multiple calls to
       123 |     // LoadingCache.get
       124 |     throw new UnsupportedLoadingOperationException();
       125 |   }
       126 | 

Added src/com/google/common/reflect/AbstractInvocationHandler.java (line 99, column 14, end column 23)

Replace generic exceptions with specific library exceptions or a custom exception.
        94 |    * 
        95 |    * <p>Unlike {@link #invoke}, {@code args} will never be null. When the method has no parameter,
        96 |    * an empty array is passed in.
        97 |    */
        98 |   protected abstract Object handleInvocation(Object proxy, Method method, Object[] args)
>>>     99 |       throws Throwable;
       100 | 
       101 |   /**
       102 |    * By default delegates to {@link Object#equals} so instances are only equal if they are
       103 |    * identical. {@code proxy.equals(argument)} returns true if: <ul>
       104 |    * <li>{@code proxy} and {@code argument} are of the same type

Added src/com/google/common/util/concurrent/AbstractExecutionThreadService.java (line 100, column 35, end column 44)

Replace generic exceptions with specific library exceptions or a custom exception.
        95 |   /**
        96 |    * Start the service. This method is invoked on the execution thread.
        97 |    * 
        98 |    * <p>By default this method does nothing.
        99 |    */
>>>    100 |   protected void startUp() throws Exception {}
       101 | 
       102 |   /**
       103 |    * Run the service. This method is invoked on the execution thread.
       104 |    * Implementations must respond to stop requests. You could poll for lifecycle
       105 |    * changes in a work loop:

Added src/com/google/common/util/concurrent/AbstractExecutionThreadService.java (line 116, column 40, end column 49)

Replace generic exceptions with specific library exceptions or a custom exception.
       111 |    *   }
       112 |    * </pre>
       113 |    * ...or you could respond to stop requests by implementing {@link
       114 |    * #triggerShutdown()}, which should cause {@link #run()} to return.
       115 |    */
>>>    116 |   protected abstract void run() throws Exception;
       117 | 
       118 |   /**
       119 |    * Stop the service. This method is invoked on the execution thread.
       120 |    * 
       121 |    * <p>By default this method does nothing.

Added src/com/google/common/util/concurrent/AbstractExecutionThreadService.java (line 124, column 36, end column 45)

Replace generic exceptions with specific library exceptions or a custom exception.
       119 |    * Stop the service. This method is invoked on the execution thread.
       120 |    * 
       121 |    * <p>By default this method does nothing.
       122 |    */
       123 |   // TODO: consider supporting a TearDownTestCase-like API
>>>    124 |   protected void shutDown() throws Exception {}
       125 | 
       126 |   /**
       127 |    * Invoked to request the service to stop.
       128 |    * 
       129 |    * <p>By default this method does nothing.

Added src/com/google/common/util/concurrent/AbstractIdleService.java (line 81, column 44, end column 53)

Replace generic exceptions with specific library exceptions or a custom exception.
        76 | 
        77 |   /** Constructor for use by subclasses. */
        78 |   protected AbstractIdleService() {}
        79 | 
        80 |   /** Start the service. */
>>>     81 |   protected abstract void startUp() throws Exception;
        82 | 
        83 |   /** Stop the service. */
        84 |   protected abstract void shutDown() throws Exception;
        85 | 
        86 |   /**

Added src/com/google/common/util/concurrent/AbstractIdleService.java (line 84, column 45, end column 54)

Replace generic exceptions with specific library exceptions or a custom exception.
        79 | 
        80 |   /** Start the service. */
        81 |   protected abstract void startUp() throws Exception;
        82 | 
        83 |   /** Stop the service. */
>>>     84 |   protected abstract void shutDown() throws Exception;
        85 | 
        86 |   /**
        87 |    * Returns the {@link Executor} that will be used to run this service.
        88 |    * Subclasses may override this method to use a custom {@link Executor}, which
        89 |    * may configure its worker thread with a specific name, thread group or

Added src/com/google/common/util/concurrent/AbstractScheduledService.java (line 265, column 52, end column 61)

Replace generic exceptions with specific library exceptions or a custom exception.
       260 |   /** 
       261 |    * Run one iteration of the scheduled task. If any invocation of this method throws an exception, 
       262 |    * the service will transition to the {@link Service.State#FAILED} state and this method will no 
       263 |    * longer be called.
       264 |    */
>>>    265 |   protected abstract void runOneIteration() throws Exception;
       266 | 
       267 |   /** 
       268 |    * Start the service.
       269 |    * 
       270 |    * <p>By default this method does nothing.

Added src/com/google/common/util/concurrent/AbstractScheduledService.java (line 272, column 35, end column 44)

Replace generic exceptions with specific library exceptions or a custom exception.
       267 |   /** 
       268 |    * Start the service.
       269 |    * 
       270 |    * <p>By default this method does nothing.
       271 |    */
>>>    272 |   protected void startUp() throws Exception {}
       273 | 
       274 |   /**
       275 |    * Stop the service. This is guaranteed not to run concurrently with {@link #runOneIteration}.
       276 |    * 
       277 |    * <p>By default this method does nothing. 

Added src/com/google/common/util/concurrent/AbstractScheduledService.java (line 279, column 36, end column 45)

Replace generic exceptions with specific library exceptions or a custom exception.
       274 |   /**
       275 |    * Stop the service. This is guaranteed not to run concurrently with {@link #runOneIteration}.
       276 |    * 
       277 |    * <p>By default this method does nothing. 
       278 |    */
>>>    279 |   protected void shutDown() throws Exception {}
       280 | 
       281 |   /**
       282 |    * Returns the {@link Scheduler} object used to configure this service.  This method will only be
       283 |    * called once. 
       284 |    */

Added src/com/google/common/util/concurrent/AsyncFunction.java (line 43, column 55, end column 64)

Replace generic exceptions with specific library exceptions or a custom exception.
        38 |    * done}, making {@code AsyncFunction} suitable for asynchronous derivations.
        39 |    *
        40 |    * <p>Throwing an exception from this method is equivalent to returning a
        41 |    * failing {@code Future}.
        42 |    */
>>>     43 |   ListenableFuture<O> apply(@Nullable I input) throws Exception;
        44 | }

Added src/com/google/common/util/concurrent/FutureFallback.java (line 51, column 50, end column 59)

Replace generic exceptions with specific library exceptions or a custom exception.
        46 |    *
        47 |    * @param t the exception that made the future fail. If the future's {@link Future#get() get}
        48 |    *     method throws an {@link ExecutionException}, then the cause is passed to this method. Any
        49 |    *     other thrown object is passed unaltered.
        50 |    */
>>>     51 |   ListenableFuture<V> create(Throwable t) throws Exception;
        52 | }

Added src/com/google/common/util/concurrent/Futures.java (line 793, column 62, end column 71)

Replace generic exceptions with specific library exceptions or a custom exception.
       788 |         setException(e);
       789 |       }
       790 |     }
       791 | 
       792 |     /** Template method for subtypes to actually run the fallback. */
>>>    793 |     abstract void doFallback(F fallback, X throwable) throws Exception;
       794 | 
       795 |     @Override final void done() {
       796 |       maybePropagateCancellation(inputFuture);
       797 |       this.inputFuture = null;
       798 |       this.exceptionType = null;
S3626 (java) on guava - 2 issues removed, 2 issues added

Removed src/com/google/common/collect/ComputingConcurrentHashMap.java (line 165, column 11, end column 26)

Remove this redundant continue.
       160 |           if (value != null) {
       161 |             recordRead(e);
       162 |             return value;
       163 |           }
       164 |           // else computing thread will clearValue
>>>    165 |           continue outer;
       166 |         }
       167 |       } finally {
       168 |         postReadCleanup();
       169 |       }
       170 |     }

Added src/com/google/common/collect/ComputingConcurrentHashMap.java (line 165, column 11, end column 26)

Remove this redundant jump.
       160 |           if (value != null) {
       161 |             recordRead(e);
       162 |             return value;
       163 |           }
       164 |           // else computing thread will clearValue
>>>    165 |           continue outer;
       166 |         }
       167 |       } finally {
       168 |         postReadCleanup();
       169 |       }
       170 |     }

Removed src/com/google/common/io/Files.java (line 742, column 9, end column 18)

Remove this redundant continue.
       737 |     List<String> path = new ArrayList<String>();
       738 | 
       739 |     // resolve ., .., and //
       740 |     for (String component : components) {
       741 |       if (component.equals(".")) {
>>>    742 |         continue;
       743 |       } else if (component.equals("..")) {
       744 |         if (path.size() > 0 && !path.get(path.size() - 1).equals("..")) {
       745 |           path.remove(path.size() - 1);
       746 |         } else {
       747 |           path.add("..");

Added src/com/google/common/io/Files.java (line 742, column 9, end column 18)

Remove this redundant jump.
       737 |     List<String> path = new ArrayList<String>();
       738 | 
       739 |     // resolve ., .., and //
       740 |     for (String component : components) {
       741 |       if (component.equals(".")) {
>>>    742 |         continue;
       743 |       } else if (component.equals("..")) {
       744 |         if (path.size() > 0 && !path.get(path.size() - 1).equals("..")) {
       745 |           path.remove(path.size() - 1);
       746 |         } else {
       747 |           path.add("..");

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6447

@romainbrenguier

Copy link
Copy Markdown
Contributor Author

Validation succeeded: Ruling Diff Comment passed, the Linux SARIF artifact and HTML diff uploads passed, and Ruling Update and Notify created automatic baseline PR #6447. The synthetic reports pass the LITS SARIF 2.1.0 schema. Ruling comparison failures are intentional because this draft does not contain regenerated baselines. Keep this smoke-test stack out of #6445.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6447

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6447

github-actions Bot and others added 7 commits November 4, 2026 08:42
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@romainbrenguier
romainbrenguier force-pushed the romain/rc-451-sarif-smoke-test branch from 35b147d to 61fcaad Compare November 4, 2026 11:53
@gitar-bot

gitar-bot Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
CI failed: Ruling-test mismatch in the RC-451 SARIF smoke-test PR: JavaRulingTest fails on Windows for guava (23 differences), eclipse_jetty_incremental (6), and apache_commons_beanutils (2), likely tied to the edited expected SARIF fixtures; a Linux ruling log is truncated with no visible failure.

Overview

Two unique error templates across 13 logs. The Windows ruling job shows a real JavaRulingTest failure (3 of 7 tests), classified as test-related and possibly change-related. The Linux ruling job excerpt shows no visible failure, so its status is unclear because the relevant section is truncated.

Failures

JavaRulingTest assertNoDifferences mismatches on Windows (confidence: medium)

  • Type: test
  • Affected jobs: 126557422045
  • Related to change: yes
  • Root cause: Surefire reports Tests run: 7, Failures: 3. guava fails at JavaRulingTest.java:223 with Issues differences: 23, eclipse_jetty_incremental fails at line 270 with Issues differences: 6, and apache_commons_beanutils fails at line 235 with Issues differences: 2. All three fail in assertNoDifferences (line 569). The analyses completed, so this is a fixture mismatch rather than a build or infrastructure failure. The PR's Guava S112 and commons-beanutils S106/S9438 SARIF changes are consistent with the mismatches. The eclipse-jetty six-difference failure is not explained by the supplied diff, and the individual differing issues are not in the logs.
  • Suggested fix: Download the actual_warp-custom-windows-2022-l_without-sonarqube-project artifact and the generated difference reports (its/ruling/target/*_differences). Reconcile the Guava S112 and commons-beanutils S106/S9438 expected SARIF with the intended smoke-test results. Investigate the eclipse-jetty incremental baseline separately, since no changed fixture explains its six differences.

Linux ruling job: no visible failure in truncated log (confidence: low)

  • Type: other
  • Affected jobs: 126557421945
  • Related to change: unclear
  • Root cause: The visible portion shows successful setup, the Generate Diff Report step, and the Upload Diff Report step. The ruling test execution and final job conclusion are in the omitted section, so no failing test or error is identified.
  • Suggested fix: Retrieve the full untruncated log, especially the ruling test output and job conclusion. If it shows fixture mismatches, compare the generated SARIF with the updated expected files.

Summary

  • Change-related failures: 1 failure group (3 JavaRulingTest cases on Windows, related_to_pr=yes), tied to the expected SARIF fixture edits. The eclipse-jetty cause is not established by the supplied diff.
  • Infrastructure/flaky failures: None identified.
  • Recommended action: Inspect the Windows ruling artifact and difference reports, reconcile the Guava S112 and commons-beanutils S106/S9438 expected SARIF, and verify the Linux job's full log for any hidden failure. The PR is a draft smoke test, so these mismatches may be expected until the fixtures are updated.
Code Review ✅ Approved

🟡 Medium risk · Replacing expected SARIF findings with synthetic smoke-test data changes ruling comparisons and could conceal discrepancies in generated analyzer results.

Adds two synthetic SARIF 2.1.0 smoke-test reports for S106 and S112 under the expected/ paths to exercise the ruling actions, using existing commons-beanutils and guava source locations. No issues found.

Verification Rules 1/1 met PRs 4 inspected

Functional validation No results

Rules 1 passed 2 not applicable

✅ Repository license compliance

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

PR 6445: RC-451 Migrate Java ruling expectations to SARIF Inspected
PR 85: RC-354: Expectation update tool improvements Inspected
PR 203: LITS-87 Preserve precise issue locations in SARIF baselines Inspected
PR 6418: RC-353 Test Sarif diff Inspected

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

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 4, 2026

Copy link
Copy Markdown
Contributor

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6447

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.

1 participant