[PR] Add null check for artifact parameter in evaluateFileNameMapping [maven-mapping]

["elharo (via GitHub)" <gi...@apache.org>]

elharo opened a new pull request, #72:
URL: https://github.com/apache/maven-mapping/pull/72

   ## Problem
   
   `MappingUtils.evaluateFileNameMapping(String, Artifact)` throws an opaque `NullPointerException` when `artifact` is null, making it difficult for callers to diagnose the issue:
   
   ```
   NullPointerException: Cannot invoke "Artifact.getArtifactHandler()" because "artifact" is null
   ```
   
   ## Fix
   
   Added an explicit null check at the start of `evaluateFileNameMapping` that throws `IllegalArgumentException` with a clear message:
   
   ```java
   if (artifact == null) {
       throw new IllegalArgumentException("artifact cannot be null");
   }
   ```
   
   ## Test
   
   Added `mappingWithNullArtifact` which verifies that `IllegalArgumentException` is thrown when null is passed for the artifact parameter.
   
   All 7 tests pass.
   
   Closes #62
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Add null check for artifact parameter in evaluateFileNameMapping [maven-mapping]

["Copilot (via GitHub) via issues" <is...@maven.apache.org>]

Copilot commented on code in PR #72:
URL: https://github.com/apache/maven-mapping/pull/72#discussion_r4442955773


##########
src/main/java/org/apache/maven/shared/mapping/MappingUtils.java:
##########
@@ -86,6 +86,9 @@ private MappingUtils() {
      * @return expression the expression to be evaluated
      */
     public static String evaluateFileNameMapping(String expression, Artifact artifact) throws InterpolationException {
+        if (artifact == null) {
+            throw new IllegalArgumentException("artifact cannot be null");
+        }

Review Comment:
   This changes the method’s failure contract from an incidental `NullPointerException` to an explicit `IllegalArgumentException`. That’s an improvement, but it’s still a behavior change that should be documented in the public API docs for `evaluateFileNameMapping` (e.g., add an `@throws IllegalArgumentException` explaining the null constraint). This makes the new contract clear for callers and aligns with JavaDoc expectations.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org