#523 Add validators for changes file (#557)

doc/changes/changes_4.2.1.md

@@ -4,6 +4,10 @@ Code name:
 
 ## Summary
 
+## Features
+
+* #523: Added validation steps for changes file
+
 ## Bugfixes
 
 * #546: Updated template for file `.settings/org.eclipse.jdt.core.prefs`

doc/requirements/design.md

@@ -920,6 +920,23 @@ Covers:
 
 Needs: impl, utest, itest
 
+#### PK Mode `verify-release` Verifies Changes File
+`dsn~verify-release-mode.verify-changes-file~1`
+
+PK's `verify-release` mode verifies that the following parts of the current changes file (e.g. `doc/changes/changes_4.2.1.md`) are present:
+* Project name
+* Version
+* Code name (= release title)
+* Summary
+
+Rationale:
+This ensures that the required information for the GitHub release notes are available.
+
+Covers:
+* [`dsn~verify-release-mode~1`](#pk-mode-verify-release)
+
+Needs: impl, utest
+
 #### PK Mode `verify-release` Checks All Issues are Closed
 `dsn~verify-release-mode.verify-issues-closed~1`
 

project-keeper/error_code_config.yml

@@ -2,4 +2,4 @@ error-tags:
   PK-CORE:
     packages:
       - com.exasol.projectkeeper
-    highest-index: 196
+    highest-index: 197

project-keeper/src/main/java/com/exasol/projectkeeper/validators/AbstractFileValidator.java

@@ -17,7 +17,8 @@
  */
 public abstract class AbstractFileValidator implements Validator {
     private final Path absoluteFilePath;
-    private final Path relativeFilePath;
+    /** Project-directory relative path to the validated file. This is useful for displaying in error messages. */
+    protected final Path relativeFilePath;
 
     /**
      * Create a new instance of {@link AbstractFileValidator}.
@@ -60,7 +61,7 @@ private ValidationFinding getMissingFileFinding() {
      * @return method (closure)
      */
     protected SimpleValidationFinding.Fix getCreateFileFix() {
-        return (Logger log) -> {
+        return (final Logger log) -> {
             try {
                 Files.createDirectories(this.absoluteFilePath.getParent());
                 writeTemplateFile(this.absoluteFilePath);

project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileValidator.java

@@ -2,15 +2,17 @@
 
 import java.nio.file.Path;
 import java.time.LocalDateTime;
-import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
 
 import com.exasol.errorreporting.ExaError;
 import com.exasol.projectkeeper.Logger;
 import com.exasol.projectkeeper.shared.ExasolVersionMatcher;
 import com.exasol.projectkeeper.sources.AnalyzedSource;
 import com.exasol.projectkeeper.validators.AbstractFileValidator;
 import com.exasol.projectkeeper.validators.finding.SimpleValidationFinding;
+import com.exasol.projectkeeper.validators.finding.SimpleValidationFinding.Fix;
 import com.exasol.projectkeeper.validators.finding.ValidationFinding;
 
 /**
@@ -24,6 +26,7 @@ public class ChangesFileValidator extends AbstractFileValidator {
     private final String projectName;
     private final List<AnalyzedSource> sources;
     private final String projectVersion;
+    private final ChangesFileIO changesFileIO;
 
     /**
      * Create a new instance of {@link ChangesFileValidator}
@@ -35,42 +38,88 @@ public class ChangesFileValidator extends AbstractFileValidator {
      */
     public ChangesFileValidator(final String projectVersion, final String projectName, final Path projectDirectory,
             final List<AnalyzedSource> sources) {
+        this(projectVersion, projectName, projectDirectory, sources, new ChangesFileIO());
+    }
+
+    ChangesFileValidator(final String projectVersion, final String projectName, final Path projectDirectory,
+            final List<AnalyzedSource> sources, final ChangesFileIO changesFileIO) {
         super(projectDirectory, ChangesFile.getPathForVersion(projectVersion));
         this.projectVersion = projectVersion;
         this.projectName = projectName;
         this.sources = sources;
+        this.changesFileIO = changesFileIO;
     }
 
     @Override
     protected void writeTemplateFile(final Path target) {
-        new ChangesFileIO().write(getTemplate(), target);
+        changesFileIO.write(getTemplate(), target);
     }
 
     @Override
     protected boolean isValidationEnabled() {
         return !new ExasolVersionMatcher().isSnapshotVersion(this.projectVersion);
     }
 
+    // [impl->dsn~verify-release-mode.verify-changes-file~1]
     @Override
     protected List<ValidationFinding> validateContent(final Path file) {
-        final var changesFile = new ChangesFileIO().read(file);
-        final var fixedSections = fixSections(changesFile);
-        if (!changesFile.equals(fixedSections)) {
-            return List.of(getWrongContentFinding(fixedSections, file));
+        final ChangesFile changesFile = changesFileIO.read(file);
+        return Stream
+                .of(validateDependencySection(file, changesFile), validateSummarySection(changesFile),
+                        validateProjectName(changesFile))
+                .filter(Optional::isPresent) //
+                .map(Optional::get) //
+                .toList();
+    }
+
+    private Optional<ValidationFinding> validateDependencySection(final Path file, final ChangesFile changesFile) {
+        final ChangesFile fixed = fixDependencyUpdateSection(changesFile);
+        if (changesFile.equals(fixed)) {
+            return noFinding();
+        } else {
+            return outdatedDependencySectionFinding(fixed, file);
+        }
+    }
+
+    private Optional<ValidationFinding> outdatedDependencySectionFinding(final ChangesFile fixedSections,
+            final Path file) {
+        final String errorMessage = ExaError.messageBuilder("E-PK-CORE-40")
+                .message("Changes file is invalid.\nExpected content:\n{{expected content}}")
+                .parameter("expected content", fixedSections.toString()).toString();
+        final Fix fix = (final Logger log) -> changesFileIO.write(fixedSections, file);
+        return Optional.of(SimpleValidationFinding.withMessage(errorMessage).andFix(fix).build());
+    }
+
+    private Optional<ValidationFinding> validateSummarySection(final ChangesFile changesFile) {
+        final Optional<ChangesFileSection> summary = changesFile.getSummarySection();
+        if (summary.isEmpty()) {
+            return finding(ExaError.messageBuilder("E-PK-CORE-193")
+                    .message("Section '## Summary' is missing in {{path}}.", relativeFilePath)
+                    .mitigation("Add section.").toString());
+        } else {
+            return noFinding();
+        }
+    }
+
+    private Optional<ValidationFinding> validateProjectName(final ChangesFile changesFile) {
+        if (changesFile.getProjectName() == null || changesFile.getProjectName().isBlank()) {
+            return finding(ExaError.messageBuilder("E-PK-CORE-195")
+                    .message("Project name in {{path}} is missing.", relativeFilePath).mitigation("Add a project name.")
+                    .toString());
         } else {
-            return Collections.emptyList();
+            return noFinding();
         }
     }
 
-    private ValidationFinding getWrongContentFinding(final ChangesFile fixedSections, final Path file) {
-        return SimpleValidationFinding
-                .withMessage(ExaError.messageBuilder("E-PK-CORE-40")
-                        .message("Changes file is invalid.\nExpected content:\n{{expected content}}")
-                        .parameter("expected content", fixedSections.toString()).toString())
-                .andFix(((final Logger log) -> new ChangesFileIO().write(fixedSections, file))).build();
+    private Optional<ValidationFinding> finding(final String message) {
+        return Optional.of(SimpleValidationFinding.withMessage(message).build());
+    }
+
+    private Optional<ValidationFinding> noFinding() {
+        return Optional.empty();
     }
 
-    private ChangesFile fixSections(final ChangesFile changesFile) {
+    private ChangesFile fixDependencyUpdateSection(final ChangesFile changesFile) {
         final var dependencySectionFixer = new DependencySectionFixer(this.sources);
         return dependencySectionFixer.fix(changesFile);
     }
@@ -83,6 +132,6 @@ private ChangesFile getTemplate() {
                 .summary(ChangesFileSection.builder("## Summary").build())
                 .addSection(ChangesFileSection.builder(FEATURES_SECTION).addLines("", FIXED_ISSUE_TEMPLATE, "").build()) //
                 .build();
-        return fixSections(changesFile);
+        return fixDependencyUpdateSection(changesFile);
     }
 }

project-keeper/src/main/java/com/exasol/projectkeeper/validators/release/ChangesFileReleaseValidator.java

@@ -1,6 +1,6 @@
 package com.exasol.projectkeeper.validators.release;
 
-import static java.util.Collections.emptyList;
+import static java.util.function.Predicate.not;
 import static java.util.stream.Collectors.toSet;
 
 import java.nio.file.Path;
@@ -10,8 +10,7 @@
 
 import com.exasol.errorreporting.ExaError;
 import com.exasol.projectkeeper.Validator;
-import com.exasol.projectkeeper.validators.changesfile.ChangesFile;
-import com.exasol.projectkeeper.validators.changesfile.FixedIssue;
+import com.exasol.projectkeeper.validators.changesfile.*;
 import com.exasol.projectkeeper.validators.finding.SimpleValidationFinding;
 import com.exasol.projectkeeper.validators.finding.ValidationFinding;
 import com.exasol.projectkeeper.validators.release.github.GitHubAdapter;
@@ -36,14 +35,46 @@ class ChangesFileReleaseValidator implements Validator {
         this.clock = clock;
     }
 
+    // [impl->dsn~verify-release-mode.verify-changes-file~1]
     @Override
     public List<ValidationFinding> validate() {
-        return Stream.of(validateReleaseDate(), validateIssuesClosed()) //
-                .flatMap(List::stream).toList();
+        return Stream.of(validateReleaseDate(), validateIssuesClosed(), validateSummary(), validateCodeName()) //
+                .filter(Optional::isPresent) //
+                .map(Optional::get) //
+                .toList();
+    }
+
+    private Optional<ValidationFinding> validateSummary() {
+        final Optional<ChangesFileSection> summarySection = changesFile.getSummarySection();
+        if (summarySection.isEmpty()) {
+            return noFindings(); // Already checked in ChangesFileValidator
+        }
+        if (hasEmptyContent(summarySection)) {
+            return finding(ExaError.messageBuilder("E-PK-CORE-194")
+                    .message("Section '## Summary' is empty in {{path}}.", changesFilePath)
+                    .mitigation("Add content to section.").toString());
+        }
+        return noFindings();
+    }
+
+    private boolean hasEmptyContent(final Optional<ChangesFileSection> summarySection) {
+        return summarySection.orElseThrow().getContent().stream() //
+                .filter(not(String::isBlank)) //
+                .findAny() //
+                .isEmpty();
+    }
+
+    private Optional<ValidationFinding> validateCodeName() {
+        if (changesFile.getCodeName() == null || changesFile.getCodeName().isBlank()) {
+            return finding(ExaError.messageBuilder("E-PK-CORE-197")
+                    .message("Code name in {{path}} is missing.", changesFilePath).mitigation("Add a code name.")
+                    .toString());
+        }
+        return noFindings();
     }
 
     // [impl->dsn~verify-release-mode.verify-release-date~1]
-    private List<ValidationFinding> validateReleaseDate() {
+    private Optional<ValidationFinding> validateReleaseDate() {
         final Optional<LocalDate> releaseDate = changesFile.getParsedReleaseDate();
         if (releaseDate.isEmpty()) {
             return finding(ExaError.messageBuilder("E-PK-CORE-182")
@@ -62,7 +93,7 @@ private List<ValidationFinding> validateReleaseDate() {
     }
 
     // [impl->dsn~verify-release-mode.verify-issues-closed~1]
-    private List<ValidationFinding> validateIssuesClosed() {
+    private Optional<ValidationFinding> validateIssuesClosed() {
         final List<Integer> wrongIssues = getIssuesWronglyMarkedAsClosed();
         if (!wrongIssues.isEmpty()) {
             return finding(ExaError.messageBuilder("E-PK-CORE-186").message(
@@ -95,11 +126,11 @@ private LocalDate today() {
         return LocalDate.ofInstant(clock.instant(), UTC_ZONE);
     }
 
-    private List<ValidationFinding> noFindings() {
-        return emptyList();
+    private Optional<ValidationFinding> noFindings() {
+        return Optional.empty();
     }
 
-    private List<ValidationFinding> finding(final String message) {
-        return List.of(SimpleValidationFinding.withMessage(message).build());
+    private Optional<ValidationFinding> finding(final String message) {
+        return Optional.of(SimpleValidationFinding.withMessage(message).build());
     }
 }