Skip to content

Commit e094931

Browse files
Throw MavenParsingException when a POM is missing groupId or version
RawPom.toPom previously called Objects.requireNonNull on the project's groupId and version, producing a context-free NullPointerException when either coordinate was absent. The NPE bubbled all the way out of mod build / parse without naming the offending artifact, making diagnosis painful. Treat blank (null or whitespace-only) values as missing in getGroupId / getVersion, and throw MavenParsingException naming the artifact and source repository when either coordinate cannot be resolved from the POM or its <parent> element.
1 parent e156df4 commit e094931

2 files changed

Lines changed: 133 additions & 15 deletions

File tree

rewrite-maven/src/main/java/org/openrewrite/maven/internal/RawPom.java

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import java.util.ArrayList;
3737
import java.util.List;
3838
import java.util.Map;
39-
import java.util.Objects;
4039

4140
import static java.util.Collections.emptyList;
4241
import static java.util.Collections.emptyMap;
@@ -404,22 +403,26 @@ public static class Profile {
404403
}
405404

406405
public @Nullable String getGroupId() {
407-
return groupId == null && parent != null ? parent.getGroupId() : groupId;
406+
if (!StringUtils.isBlank(groupId)) {
407+
return groupId;
408+
}
409+
if (parent != null && !StringUtils.isBlank(parent.getGroupId())) {
410+
return parent.getGroupId();
411+
}
412+
return null;
408413
}
409414

410415
public @Nullable String getVersion() {
411-
if (version == null) {
412-
if (currentVersion == null) {
413-
if (parent == null) {
414-
return null;
415-
} else {
416-
return parent.getVersion();
417-
}
418-
} else {
419-
return currentVersion;
420-
}
416+
if (!StringUtils.isBlank(version)) {
417+
return version;
421418
}
422-
return version;
419+
if (!StringUtils.isBlank(currentVersion)) {
420+
return currentVersion;
421+
}
422+
if (parent != null && !StringUtils.isBlank(parent.getVersion())) {
423+
return parent.getVersion();
424+
}
425+
return null;
423426
}
424427

425428

@@ -428,15 +431,26 @@ public Pom toPom(@Nullable Path inputPath, @Nullable MavenRepository repo) {
428431
getParent().getGroupId(), getParent().getArtifactId(),
429432
getParent().getVersion()), getParent().getRelativePath());
430433

434+
String resolvedGroupId = getGroupId();
435+
String resolvedVersion = getVersion();
436+
if (resolvedGroupId == null || resolvedVersion == null) {
437+
throw new MavenParsingException(
438+
"POM is missing a required coordinate:" +
439+
(resolvedGroupId == null ? " groupId" : "") +
440+
(resolvedVersion == null ? " version" : "") +
441+
" for " + new GroupArtifactVersion(resolvedGroupId, artifactId, resolvedVersion) +
442+
(repo == null ? "" : " (from " + repo.getUri() + ")"));
443+
}
444+
431445
Pom.PomBuilder builder = Pom.builder()
432446
.sourcePath(inputPath)
433447
.repository(repo)
434448
.parent(parent)
435449
.gav(new ResolvedGroupArtifactVersion(
436450
repo == null ? null : repo.getUri(),
437-
Objects.requireNonNull(getGroupId()),
451+
resolvedGroupId,
438452
artifactId,
439-
Objects.requireNonNull(getVersion()),
453+
resolvedVersion,
440454
null))
441455
.name(name)
442456
.obsoletePomVersion(pomVersion)

rewrite-maven/src/test/java/org/openrewrite/maven/internal/RawPomTest.java

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,4 +610,108 @@ void deserializePluginConfiguration() throws Exception {
610610
assertThat(plugin.getConfigurationList("grandparent.parent.child.stringList", String.class)).hasSize(4)
611611
.contains("f", "r", "e", "d");
612612
}
613+
614+
@Test
615+
void missingGroupIdThrowsParsingException() {
616+
RawPom pom = RawPom.parse(
617+
//language=xml
618+
new ByteArrayInputStream("""
619+
<project>
620+
<modelVersion>4.0.0</modelVersion>
621+
<artifactId>my-app</artifactId>
622+
<version>1</version>
623+
</project>
624+
""".getBytes()),
625+
null
626+
);
627+
628+
assertThatThrownBy(() -> pom.toPom(null, null))
629+
.isInstanceOf(MavenParsingException.class)
630+
.hasMessageContaining("missing a required coordinate")
631+
.hasMessageContaining("groupId")
632+
.hasMessageContaining("my-app");
633+
}
634+
635+
@Test
636+
void missingVersionThrowsParsingException() {
637+
RawPom pom = RawPom.parse(
638+
//language=xml
639+
new ByteArrayInputStream("""
640+
<project>
641+
<modelVersion>4.0.0</modelVersion>
642+
<groupId>com.mycompany.app</groupId>
643+
<artifactId>my-app</artifactId>
644+
</project>
645+
""".getBytes()),
646+
null
647+
);
648+
649+
assertThatThrownBy(() -> pom.toPom(null, null))
650+
.isInstanceOf(MavenParsingException.class)
651+
.hasMessageContaining("missing a required coordinate")
652+
.hasMessageContaining("version");
653+
}
654+
655+
@Test
656+
void emptyGroupIdElementThrowsParsingException() {
657+
RawPom pom = RawPom.parse(
658+
//language=xml
659+
new ByteArrayInputStream("""
660+
<project>
661+
<modelVersion>4.0.0</modelVersion>
662+
<groupId/>
663+
<artifactId>my-app</artifactId>
664+
<version>1</version>
665+
</project>
666+
""".getBytes()),
667+
null
668+
);
669+
670+
assertThatThrownBy(() -> pom.toPom(null, null))
671+
.isInstanceOf(MavenParsingException.class)
672+
.hasMessageContaining("groupId");
673+
}
674+
675+
@Test
676+
void whitespaceVersionElementThrowsParsingException() {
677+
RawPom pom = RawPom.parse(
678+
//language=xml
679+
new ByteArrayInputStream("""
680+
<project>
681+
<modelVersion>4.0.0</modelVersion>
682+
<groupId>com.mycompany.app</groupId>
683+
<artifactId>my-app</artifactId>
684+
<version> </version>
685+
</project>
686+
""".getBytes()),
687+
null
688+
);
689+
690+
assertThatThrownBy(() -> pom.toPom(null, null))
691+
.isInstanceOf(MavenParsingException.class)
692+
.hasMessageContaining("version");
693+
}
694+
695+
@Test
696+
void groupIdInheritedFromParentStillResolves() {
697+
RawPom pom = RawPom.parse(
698+
//language=xml
699+
new ByteArrayInputStream("""
700+
<project>
701+
<modelVersion>4.0.0</modelVersion>
702+
<parent>
703+
<groupId>com.mycompany.app</groupId>
704+
<artifactId>my-parent</artifactId>
705+
<version>1</version>
706+
</parent>
707+
<artifactId>my-app</artifactId>
708+
</project>
709+
""".getBytes()),
710+
null
711+
);
712+
713+
Pom resolved = pom.toPom(null, null);
714+
assertThat(resolved.getGroupId()).isEqualTo("com.mycompany.app");
715+
assertThat(resolved.getVersion()).isEqualTo("1");
716+
}
613717
}

0 commit comments

Comments
 (0)