Skip to content

fix: warn when --sources/--files are used with executable archives#2479

Open
maxandersen wants to merge 1 commit into
mainfrom
2363-warn-sources-ignored-for-jars
Open

fix: warn when --sources/--files are used with executable archives#2479
maxandersen wants to merge 1 commit into
mainfrom
2363-warn-sources-ignored-for-jars

Conversation

@maxandersen
Copy link
Copy Markdown
Collaborator

@maxandersen maxandersen commented May 27, 2026

Summary

  • When running a JAR/WAR file with --sources or --files, these options were silently ignored
  • Now emits a [WARN] message and skips source/resource resolution for executable archives
  • Warning is placed in ProjectBuilder.updateProject() so it fires before source file resolution (which would otherwise fail for nonexistent files)

Fixes #2363

Test plan

  • Added testJarWithSourcesWarning test in TestRun.java
  • Existing testHelloWorldJar still passes
  • Manual: jbang run --sources Foo.java some.jar prints warning to stderr

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added warnings when users attempt to use --sources or --files options with executable archives (jar/war files), as these options are not supported for this archive type.
  • Tests

    • Added test coverage for executable archive handling.

Review Change Stack

When running a JAR/WAR file with --sources or --files, these options
were silently ignored. Now emits a warning and skips source/resource
resolution for executable archives.

Fixes #2363

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

ProjectBuilder.updateProject() now detects when a project is an executable archive and skips adding user-specified additional sources and resources while emitting warnings that these options are unsupported. For non-executable archives, the prior behavior is preserved. A new test verifies the warning is emitted for executable JARs.

Changes

Executable Archive Warning

Layer / File(s) Summary
Executable archive detection and warning logic
src/main/java/dev/jbang/source/ProjectBuilder.java
updateProject() branches on whether the project is an executable archive: for jar/war archives, additional sources and resources are skipped with warnings; for non-executable archives, prior behavior is preserved by adding sources via updateAllSources() and converting resources to RefTarget references.
Test for --sources warning on executable JAR
src/test/java/dev/jbang/cli/TestRun.java
New testJarWithSourcesWarning test runs run for hellojar.jar with --sources and asserts that stderr contains the expected warning.

🎯 2 (Simple) | ⏱️ ~8 minutes

A jar cried out in alarm,
"Your sources bring no harm,
But archives like me,
Can't use --sources, you see—
I warn you now, tis the charm!" 🐰📦

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding warnings when --sources/--files are used with executable archives, which is the primary objective of this PR.
Linked Issues check ✅ Passed The PR implements the desired behavior from issue #2363 by emitting a warning when --sources/--files are used with executable archives, rather than silently ignoring them.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue #2363: ProjectBuilder branching logic for archives, warning emission, and corresponding test for the warning behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2363-warn-sources-ignored-for-jars

Comment @coderabbitai help to get the list of available commands and usage tips.

@maxandersen maxandersen marked this pull request as ready for review May 27, 2026 22:46
@maxandersen
Copy link
Copy Markdown
Collaborator Author

@quintesse wdyt ? going for warn about files /sources not being honored in this pr - mainly because I can't see a (good) usecase where we want to "reopen" the jar and add the resources....do you know of one or is this good enough

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/test/java/dev/jbang/cli/TestRun.java (1)

371-376: ⚡ Quick win

Consider adding test coverage for --files warning.

The test correctly verifies the warning for --sources, but there's no corresponding test for the --files option. Consider adding a similar test case to ensure both code paths are covered.

📋 Suggested additional test
`@Test`
void testJarWithFilesWarning() throws Exception {
	String jar = examplesTestFolder.resolve("hellojar.jar").toAbsolutePath().toString();
	CaptureResult<Integer> result = checkedRun(null, "run", "--files", "SomeFile.txt", jar);
	assertThat(result.err, containsString("--files option is not supported for executable archives"));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test/java/dev/jbang/cli/TestRun.java` around lines 371 - 376, Add a new
JUnit test mirroring testJarWithSourcesWarning to cover the `--files` code path:
create a method named testJarWithFilesWarning in the same TestRun class, build
the jar path using
examplesTestFolder.resolve("hellojar.jar").toAbsolutePath().toString(), call
checkedRun(null, "run", "--files", "SomeFile.txt", jar) and assert that
result.err contains the message "--files option is not supported for executable
archives"; use the same CaptureResult<Integer> and assertion style as
testJarWithSourcesWarning so both warnings are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/test/java/dev/jbang/cli/TestRun.java`:
- Around line 371-376: Add a new JUnit test mirroring testJarWithSourcesWarning
to cover the `--files` code path: create a method named testJarWithFilesWarning
in the same TestRun class, build the jar path using
examplesTestFolder.resolve("hellojar.jar").toAbsolutePath().toString(), call
checkedRun(null, "run", "--files", "SomeFile.txt", jar) and assert that
result.err contains the message "--files option is not supported for executable
archives"; use the same CaptureResult<Integer> and assertion style as
testJarWithSourcesWarning so both warnings are covered.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 84086097-8f2f-410b-9e2d-bf706b47f1d8

📥 Commits

Reviewing files that changed from the base of the PR and between 0c1b40a and ee99759.

📒 Files selected for processing (2)
  • src/main/java/dev/jbang/source/ProjectBuilder.java
  • src/test/java/dev/jbang/cli/TestRun.java

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.

Running a jar file silently ignores --sources

1 participant