[#164] fix error found in #164 #165

bmarwell · 2025-11-17T14:47:37Z

No description provided.

codecov · 2025-11-17T14:48:33Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
33	1	32	0

View the top 1 failed test(s) by shortest run time

io.github.bmarwell.jfmt.imports.Issue164Test::is_correctly_formatted

Stack Traces | 0.158s run time

ImportOrderProcessorException[super=io.github.bmarwell.jfmt.imports.ImportOrderProcessorException: Problem formatting file [.../cli/imports/Issue164TestDatasource.java]. Range: [35-723]. Rendered old: >>>
import invalid.tool.dao.JpaUserDao;
import invalid.tool.dao.UserDao;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityTransaction;
import jakarta.persistence.Parameter;
import jakarta.persistence.Query;
import jakarta.persistence.TypedQuery;
import jakarta.persistence.criteria.CriteriaQuery;
import java.util.function.Supplier;
import org.eclipse.persistence.internal.helper.DatabaseField;
import org.eclipse.persistence.internal.jpa.EJBQueryImpl;
import org.eclipse.persistence.jpa.JpaEntityManager;
import org.eclipse.persistence.queries.DatabaseQuery;
import org.eclipse.persistence.sessions.DatabaseRecord;
import org.eclipse.persistence.sessions.Session;


<<<, path=imports/Issue164TestDatasource.java]

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

Copilot

Pull Request Overview

This PR fixes an error found in issue #164 by enhancing error handling in the import order processor. The main purpose is to provide better diagnostics when the Eclipse JDT parser encounters formatting errors during import rewriting operations.

Key changes:

Added Path parameter to ImportOrderProcessor.rewriteImportsIfAny() to track which file is being processed
Introduced ImportOrderProcessorException to wrap formatting errors with file context
Refactored test utilities to use a TestResource record that bundles path and contents together

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
cli/src/main/java/io/github/bmarwell/jfmt/imports/ImportOrderProcessorException.java	New exception class to wrap import formatting errors with file path context
cli/src/main/java/io/github/bmarwell/jfmt/commands/ImportOrderProcessor.java	Added Path parameter to track file being processed and enhanced error handling with try-catch for IllegalArgumentException
cli/src/main/java/io/github/bmarwell/jfmt/commands/AbstractCommand.java	Updated method call to pass javaFile path to ImportOrderProcessor
cli/src/test/java/io/github/bmarwell/jfmt/imports/ImportOrderTestUtil.java	Introduced TestResource record to bundle path and contents; refactored loadTestResource to return this record
cli/src/test/java/io/github/bmarwell/jfmt/imports/ImportOrderProcessorTestBase.java	Updated to use TestResource record and pass path to ImportOrderProcessor
cli/src/test/java/io/github/bmarwell/jfmt/imports/Issue164Test.java	New test case verifying import ordering for the Issue164TestDatasource test resource
cli/src/test/java/io/github/bmarwell/jfmt/imports/FallbackMatchesDefaultTest.java	Updated to use TestResource record and pass path to ImportOrderProcessor
cli/src/test/java/io/github/bmarwell/jfmt/imports/EmptyConfigurationImportOrderTest.java	Updated to use TestResource record and pass path to ImportOrderProcessor
cli/src/test/java/io/github/bmarwell/jfmt/imports/ApacheImportOrderTest.java	Updated to use simplified test resource loading through ImportOrderTestUtil
cli/src/test/resources/imports/Issue164TestDatasource.java	Test resource file with mixed imports to validate formatting behavior for issue #164

Copilot · 2025-11-17T14:51:48Z

cli/src/main/java/io/github/bmarwell/jfmt/imports/ImportOrderProcessorException.java

+
+import java.nio.file.Path;
+import java.util.StringJoiner;
+


The exception class lacks documentation. Add a JavaDoc comment describing when this exception is thrown and what it represents.

Example:

/** * Exception thrown when the import order processor encounters an error * while rewriting imports in a Java source file. */ public class ImportOrderProcessorException extends RuntimeException {

Suggested change

/**

* Exception thrown when the import order processor encounters an error

* while rewriting imports in a Java source file.

* <p>

* This exception provides the path of the file being processed and the underlying cause.

*/

Copilot · 2025-11-17T14:51:48Z

cli/src/main/java/io/github/bmarwell/jfmt/imports/ImportOrderProcessorException.java

+
+public class ImportOrderProcessorException extends RuntimeException {
+    private final Path path;
+


The constructor lacks documentation. Add a JavaDoc comment describing the parameters.

Example:

/** * Creates a new import order processor exception. * * @param path the path to the Java file being processed * @param message the error message * @param cause the underlying cause of the error */ public ImportOrderProcessorException(Path path, String message, Throwable cause) {

Suggested change

/**

* Creates a new import order processor exception.

*

* @param path the path to the Java file being processed

* @param message the error message

* @param cause the underlying cause of the error

*/

Copilot · 2025-11-17T14:51:49Z

cli/src/main/java/io/github/bmarwell/jfmt/commands/ImportOrderProcessor.java

+                javaFile.toAbsolutePath(),
+                importStart,
+                importEnd,
+                workingDoc.get().substring(importStart, importEnd)


The substring call workingDoc.get().substring(importStart, importEnd) may throw StringIndexOutOfBoundsException if importEnd exceeds the document length after the workingDoc.replace() call on line 180. This can happen because the document has already been modified before this error handling code runs.

Consider using a safer approach:

int docLength = workingDoc.get().length(); int safeEnd = Math.min(importEnd, docLength); workingDoc.get().substring(importStart, safeEnd)

Or capture the original substring before the replace operation if that's what you intend to show in the error message.

Suggested change

workingDoc.get().substring(importStart, importEnd)

workingDoc.get().substring(importStart, Math.min(importEnd, workingDoc.get().length()))

Copilot · 2025-11-17T14:51:50Z

cli/src/main/java/io/github/bmarwell/jfmt/commands/ImportOrderProcessor.java

+        } catch (IllegalArgumentException jdtEx) {
+            String message = String.format(
+                Locale.ROOT,
+                "Problem formatting file [%s]. Range: [%d-%d]. Rendered old: >>>\n%s\n<<<",
+                javaFile.toAbsolutePath(),
+                importStart,
+                importEnd,
+                workingDoc.get().substring(importStart, importEnd)
+            );
+
+            throw new ImportOrderProcessorException(javaFile, message, jdtEx);
+        }


The exception handling for IllegalArgumentException lacks test coverage. Consider adding a test case that triggers this error path to ensure the ImportOrderProcessorException is thrown correctly with the expected error message and context.

This would help verify:

The exception message format is correct

The file path is properly included in the exception

The original IllegalArgumentException is wrapped as the cause

Copilot · 2025-11-17T14:51:50Z

cli/src/test/java/io/github/bmarwell/jfmt/imports/ImportOrderTestUtil.java

+    record TestResource(Path path, String contents) {}
+


The TestResource record lacks documentation. Add a JavaDoc comment explaining its purpose and the meaning of its components.

Example:

/** * Represents a loaded test resource with its path and contents. * * @param path the resource path relative to the classpath * @param contents the file contents as a string */ record TestResource(Path path, String contents) {}

Suggested change

record TestResource(Path path, String contents) {}

Copilot · 2025-11-17T14:51:50Z

cli/src/main/java/io/github/bmarwell/jfmt/commands/ImportOrderProcessor.java


    /**
     * Rewrites the imports in the provided document according to the configured order.
     * If there are no imports, this method is a no-op.


The JavaDoc for rewriteImportsIfAny method is incomplete. It should document the new javaFile parameter and the new ImportOrderProcessorException that can be thrown.

Suggested update:

/** * Rewrites the imports in the provided document according to the configured order. * If there are no imports, this method is a no-op. * * @param javaFile * the path to the Java source file being processed * @param compilationUnit * the parsed compilation unit containing the imports * @param workingDoc * the document to modify * @throws BadLocationException * if the import block cannot be replaced at the calculated position * @throws ImportOrderProcessorException * if the import rewrite operation fails */

Suggested change

* If there are no imports, this method is a no-op.

* If there are no imports, this method is a no-op.

*

* @param javaFile

* the path to the Java source file being processed

* @param compilationUnit

* the parsed compilation unit containing the imports

* @param workingDoc

* the document to modify

* @throws BadLocationException

* if the import block cannot be replaced at the calculated position

* @throws ImportOrderProcessorException

* if the import rewrite operation fails

Copilot · 2025-11-17T14:51:50Z

cli/src/main/java/io/github/bmarwell/jfmt/commands/ImportOrderProcessor.java

+    /**
+     *
+     * @param javaFile
+     * @param compilationUnit
+     * @param workingDoc
+     * @param rendered
+     *     the newly rendered import block
+     * @throws BadLocationException


JavaDoc parameter descriptions are missing. The @param tags should include descriptions for what each parameter represents, not just the parameter name.

Suggested format:

/** * Replaces the import block in the document. * * @param javaFile * the path to the Java source file being processed * @param compilationUnit * the parsed compilation unit containing the imports * @param workingDoc * the document being modified * @param rendered * the newly rendered import block * @throws BadLocationException * if the import block cannot be replaced at the calculated position */

Suggested change

/**

*

* @param javaFile

* @param compilationUnit

* @param workingDoc

* @param rendered

* the newly rendered import block

* @throws BadLocationException

/**

* Replaces the import block in the document.

*

* @param javaFile

* the path to the Java source file being processed

* @param compilationUnit

* the parsed compilation unit containing the imports

* @param workingDoc

* the document being modified

* @param rendered

* the newly rendered import block

* @throws BadLocationException

* if the import block cannot be replaced at the calculated position

cli/src/test/resources/imports/Issue164TestDatasource.java

bmarwell · 2025-11-17T20:17:15Z

Blocked by: eclipse-jdt/eclipse.jdt.core#4128

[#164] implement test case for formatting error

8bfcbc4

Copilot AI review requested due to automatic review settings November 17, 2025 14:47

Copilot started reviewing on behalf of bmarwell November 17, 2025 14:48 View session

Copilot finished reviewing on behalf of bmarwell November 17, 2025 14:50

Copilot AI reviewed Nov 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[#164] fix error found in #164 #165

[#164] fix error found in #164 #165

Uh oh!

bmarwell commented Nov 17, 2025

Uh oh!

codecov bot commented Nov 17, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 17, 2025

Uh oh!

Copilot AI Nov 17, 2025

Uh oh!

Copilot AI Nov 17, 2025

Uh oh!

Copilot AI Nov 17, 2025

Uh oh!

Copilot AI Nov 17, 2025

Uh oh!

Copilot AI Nov 17, 2025

Uh oh!

Copilot AI Nov 17, 2025

Uh oh!

Uh oh!

bmarwell commented Nov 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

+/**
+ * Exception thrown when the import order processor encounters an error
+ * while rewriting imports in a Java source file.
+ * <p>
+ * This exception provides the path of the file being processed and the underlying cause.
+ */


		public class ImportOrderProcessorException extends RuntimeException {
		private final Path path;

+    /**
+     * Creates a new import order processor exception.
+     *
+     * @param path the path to the Java file being processed
+     * @param message the error message
+     * @param cause the underlying cause of the error
+     */

	workingDoc.get().substring(importStart, importEnd)
	workingDoc.get().substring(importStart, Math.min(importEnd, workingDoc.get().length()))

[#164] fix error found in #164 #165

Are you sure you want to change the base?

[#164] fix error found in #164 #165

Uh oh!

Conversation

bmarwell commented Nov 17, 2025

Uh oh!

codecov bot commented Nov 17, 2025

❌ 1 Tests Failed:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bmarwell commented Nov 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants