Skip to content

Conversation

@jogerj
Copy link

@jogerj jogerj commented Jun 27, 2025

  • fix version mismatch on maven Java versions aren't published #235
  • replace java code generation with lombok to make it more maintainable
    • I tried to retain the compatibility of the generated code as much as possible. The builder is fully compatible.
    • The field names are changed to camelCase (following java convention), this is largely safe to do since we only instantiate the fields from the private constructor once. However, if someone reads the fields of the configuration object directly, to them this would be a breaking change. These fields should have been private and not exposed directly anyways.
    • When the configuration is created by the builder as recommended, the behavior would be the same (same method and class names)
    • lombok generates additionally a static method Configuration.builder() and a toString method on the builder and configuration object.
    • compiled package does not include lombok dependencies
  • Refactoring
    • minor refactoring of os/arch variable checks
    • added null check in java to prevent panic
    • lib.rs calls getter methods of the Configuration POJO
    • basic tests
  • updated maven plugins versions
  • remove js script to generate java code. replace with bash script + rely on markers in Configuration.java and lib.rs

With that I'd like to know whether the breaking change above mentioned is acceptable or if anything needs to be further adjusted.

Generated class preview
//
// Source code recreated from a .class file by IntelliJ IDEA
// (powered by FernFlower decompiler)
//

package in.wilsonl.minifyhtml;

public final class Configuration {
    private final boolean allowNoncompliantUnquotedAttributeValues;
    private final boolean allowOptimalEntities;
    private final boolean allowRemovingSpacesBetweenAttributes;
    private final boolean keepClosingTags;
    private final boolean keepComments;
    private final boolean keepHtmlAndHeadOpeningTags;
    private final boolean keepInputTypeTextAttr;
    private final boolean keepSsiComments;
    private final boolean minifyCss;
    private final boolean minifyDoctype;
    private final boolean minifyJs;
    private final boolean preserveBraceTemplateSyntax;
    private final boolean preserveChevronPercentTemplateSyntax;
    private final boolean removeBangs;
    private final boolean removeProcessingInstructions;

    public static Builder builder() {
        return new Builder();
    }

    public boolean isAllowNoncompliantUnquotedAttributeValues() {
        return this.allowNoncompliantUnquotedAttributeValues;
    }

    public boolean isAllowOptimalEntities() {
        return this.allowOptimalEntities;
    }

    public boolean isAllowRemovingSpacesBetweenAttributes() {
        return this.allowRemovingSpacesBetweenAttributes;
    }

    public boolean isKeepClosingTags() {
        return this.keepClosingTags;
    }

    public boolean isKeepComments() {
        return this.keepComments;
    }

    public boolean isKeepHtmlAndHeadOpeningTags() {
        return this.keepHtmlAndHeadOpeningTags;
    }

    public boolean isKeepInputTypeTextAttr() {
        return this.keepInputTypeTextAttr;
    }

    public boolean isKeepSsiComments() {
        return this.keepSsiComments;
    }

    public boolean isMinifyCss() {
        return this.minifyCss;
    }

    public boolean isMinifyDoctype() {
        return this.minifyDoctype;
    }

    public boolean isMinifyJs() {
        return this.minifyJs;
    }

    public boolean isPreserveBraceTemplateSyntax() {
        return this.preserveBraceTemplateSyntax;
    }

    public boolean isPreserveChevronPercentTemplateSyntax() {
        return this.preserveChevronPercentTemplateSyntax;
    }

    public boolean isRemoveBangs() {
        return this.removeBangs;
    }

    public boolean isRemoveProcessingInstructions() {
        return this.removeProcessingInstructions;
    }

    public boolean equals(Object o) {
        if (o == this) {
            return true;
        } else if (!(o instanceof Configuration)) {
            return false;
        } else {
            Configuration other = (Configuration)o;
            if (this.isAllowNoncompliantUnquotedAttributeValues() != other.isAllowNoncompliantUnquotedAttributeValues()) {
                return false;
            } else if (this.isAllowOptimalEntities() != other.isAllowOptimalEntities()) {
                return false;
            } else if (this.isAllowRemovingSpacesBetweenAttributes() != other.isAllowRemovingSpacesBetweenAttributes()) {
                return false;
            } else if (this.isKeepClosingTags() != other.isKeepClosingTags()) {
                return false;
            } else if (this.isKeepComments() != other.isKeepComments()) {
                return false;
            } else if (this.isKeepHtmlAndHeadOpeningTags() != other.isKeepHtmlAndHeadOpeningTags()) {
                return false;
            } else if (this.isKeepInputTypeTextAttr() != other.isKeepInputTypeTextAttr()) {
                return false;
            } else if (this.isKeepSsiComments() != other.isKeepSsiComments()) {
                return false;
            } else if (this.isMinifyCss() != other.isMinifyCss()) {
                return false;
            } else if (this.isMinifyDoctype() != other.isMinifyDoctype()) {
                return false;
            } else if (this.isMinifyJs() != other.isMinifyJs()) {
                return false;
            } else if (this.isPreserveBraceTemplateSyntax() != other.isPreserveBraceTemplateSyntax()) {
                return false;
            } else if (this.isPreserveChevronPercentTemplateSyntax() != other.isPreserveChevronPercentTemplateSyntax()) {
                return false;
            } else if (this.isRemoveBangs() != other.isRemoveBangs()) {
                return false;
            } else {
                return this.isRemoveProcessingInstructions() == other.isRemoveProcessingInstructions();
            }
        }
    }

    public int hashCode() {
        int PRIME = 59;
        int result = 1;
        result = result * 59 + (this.isAllowNoncompliantUnquotedAttributeValues() ? 79 : 97);
        result = result * 59 + (this.isAllowOptimalEntities() ? 79 : 97);
        result = result * 59 + (this.isAllowRemovingSpacesBetweenAttributes() ? 79 : 97);
        result = result * 59 + (this.isKeepClosingTags() ? 79 : 97);
        result = result * 59 + (this.isKeepComments() ? 79 : 97);
        result = result * 59 + (this.isKeepHtmlAndHeadOpeningTags() ? 79 : 97);
        result = result * 59 + (this.isKeepInputTypeTextAttr() ? 79 : 97);
        result = result * 59 + (this.isKeepSsiComments() ? 79 : 97);
        result = result * 59 + (this.isMinifyCss() ? 79 : 97);
        result = result * 59 + (this.isMinifyDoctype() ? 79 : 97);
        result = result * 59 + (this.isMinifyJs() ? 79 : 97);
        result = result * 59 + (this.isPreserveBraceTemplateSyntax() ? 79 : 97);
        result = result * 59 + (this.isPreserveChevronPercentTemplateSyntax() ? 79 : 97);
        result = result * 59 + (this.isRemoveBangs() ? 79 : 97);
        result = result * 59 + (this.isRemoveProcessingInstructions() ? 79 : 97);
        return result;
    }

    public String toString() {
        return "Configuration(allowNoncompliantUnquotedAttributeValues=" + this.isAllowNoncompliantUnquotedAttributeValues() + ", allowOptimalEntities=" + this.isAllowOptimalEntities() + ", allowRemovingSpacesBetweenAttributes=" + this.isAllowRemovingSpacesBetweenAttributes() + ", keepClosingTags=" + this.isKeepClosingTags() + ", keepComments=" + this.isKeepComments() + ", keepHtmlAndHeadOpeningTags=" + this.isKeepHtmlAndHeadOpeningTags() + ", keepInputTypeTextAttr=" + this.isKeepInputTypeTextAttr() + ", keepSsiComments=" + this.isKeepSsiComments() + ", minifyCss=" + this.isMinifyCss() + ", minifyDoctype=" + this.isMinifyDoctype() + ", minifyJs=" + this.isMinifyJs() + ", preserveBraceTemplateSyntax=" + this.isPreserveBraceTemplateSyntax() + ", preserveChevronPercentTemplateSyntax=" + this.isPreserveChevronPercentTemplateSyntax() + ", removeBangs=" + this.isRemoveBangs() + ", removeProcessingInstructions=" + this.isRemoveProcessingInstructions() + ")";
    }

    private Configuration(boolean allowNoncompliantUnquotedAttributeValues, boolean allowOptimalEntities, boolean allowRemovingSpacesBetweenAttributes, boolean keepClosingTags, boolean keepComments, boolean keepHtmlAndHeadOpeningTags, boolean keepInputTypeTextAttr, boolean keepSsiComments, boolean minifyCss, boolean minifyDoctype, boolean minifyJs, boolean preserveBraceTemplateSyntax, boolean preserveChevronPercentTemplateSyntax, boolean removeBangs, boolean removeProcessingInstructions) {
        this.allowNoncompliantUnquotedAttributeValues = allowNoncompliantUnquotedAttributeValues;
        this.allowOptimalEntities = allowOptimalEntities;
        this.allowRemovingSpacesBetweenAttributes = allowRemovingSpacesBetweenAttributes;
        this.keepClosingTags = keepClosingTags;
        this.keepComments = keepComments;
        this.keepHtmlAndHeadOpeningTags = keepHtmlAndHeadOpeningTags;
        this.keepInputTypeTextAttr = keepInputTypeTextAttr;
        this.keepSsiComments = keepSsiComments;
        this.minifyCss = minifyCss;
        this.minifyDoctype = minifyDoctype;
        this.minifyJs = minifyJs;
        this.preserveBraceTemplateSyntax = preserveBraceTemplateSyntax;
        this.preserveChevronPercentTemplateSyntax = preserveChevronPercentTemplateSyntax;
        this.removeBangs = removeBangs;
        this.removeProcessingInstructions = removeProcessingInstructions;
    }

    public static class Builder {
        private boolean allowNoncompliantUnquotedAttributeValues;
        private boolean allowOptimalEntities;
        private boolean allowRemovingSpacesBetweenAttributes;
        private boolean keepClosingTags;
        private boolean keepComments;
        private boolean keepHtmlAndHeadOpeningTags;
        private boolean keepInputTypeTextAttr;
        private boolean keepSsiComments;
        private boolean minifyCss;
        private boolean minifyDoctype;
        private boolean minifyJs;
        private boolean preserveBraceTemplateSyntax;
        private boolean preserveChevronPercentTemplateSyntax;
        private boolean removeBangs;
        private boolean removeProcessingInstructions;

        Builder() {
        }

        public Builder setAllowNoncompliantUnquotedAttributeValues(boolean allowNoncompliantUnquotedAttributeValues) {
            this.allowNoncompliantUnquotedAttributeValues = allowNoncompliantUnquotedAttributeValues;
            return this;
        }

        public Builder setAllowOptimalEntities(boolean allowOptimalEntities) {
            this.allowOptimalEntities = allowOptimalEntities;
            return this;
        }

        public Builder setAllowRemovingSpacesBetweenAttributes(boolean allowRemovingSpacesBetweenAttributes) {
            this.allowRemovingSpacesBetweenAttributes = allowRemovingSpacesBetweenAttributes;
            return this;
        }

        public Builder setKeepClosingTags(boolean keepClosingTags) {
            this.keepClosingTags = keepClosingTags;
            return this;
        }

        public Builder setKeepComments(boolean keepComments) {
            this.keepComments = keepComments;
            return this;
        }

        public Builder setKeepHtmlAndHeadOpeningTags(boolean keepHtmlAndHeadOpeningTags) {
            this.keepHtmlAndHeadOpeningTags = keepHtmlAndHeadOpeningTags;
            return this;
        }

        public Builder setKeepInputTypeTextAttr(boolean keepInputTypeTextAttr) {
            this.keepInputTypeTextAttr = keepInputTypeTextAttr;
            return this;
        }

        public Builder setKeepSsiComments(boolean keepSsiComments) {
            this.keepSsiComments = keepSsiComments;
            return this;
        }

        public Builder setMinifyCss(boolean minifyCss) {
            this.minifyCss = minifyCss;
            return this;
        }

        public Builder setMinifyDoctype(boolean minifyDoctype) {
            this.minifyDoctype = minifyDoctype;
            return this;
        }

        public Builder setMinifyJs(boolean minifyJs) {
            this.minifyJs = minifyJs;
            return this;
        }

        public Builder setPreserveBraceTemplateSyntax(boolean preserveBraceTemplateSyntax) {
            this.preserveBraceTemplateSyntax = preserveBraceTemplateSyntax;
            return this;
        }

        public Builder setPreserveChevronPercentTemplateSyntax(boolean preserveChevronPercentTemplateSyntax) {
            this.preserveChevronPercentTemplateSyntax = preserveChevronPercentTemplateSyntax;
            return this;
        }

        public Builder setRemoveBangs(boolean removeBangs) {
            this.removeBangs = removeBangs;
            return this;
        }

        public Builder setRemoveProcessingInstructions(boolean removeProcessingInstructions) {
            this.removeProcessingInstructions = removeProcessingInstructions;
            return this;
        }

        public Configuration build() {
            return new Configuration(this.allowNoncompliantUnquotedAttributeValues, this.allowOptimalEntities, this.allowRemovingSpacesBetweenAttributes, this.keepClosingTags, this.keepComments, this.keepHtmlAndHeadOpeningTags, this.keepInputTypeTextAttr, this.keepSsiComments, this.minifyCss, this.minifyDoctype, this.minifyJs, this.preserveBraceTemplateSyntax, this.preserveChevronPercentTemplateSyntax, this.removeBangs, this.removeProcessingInstructions);
        }

        public String toString() {
            return "Configuration.Builder(allowNoncompliantUnquotedAttributeValues=" + this.allowNoncompliantUnquotedAttributeValues + ", allowOptimalEntities=" + this.allowOptimalEntities + ", allowRemovingSpacesBetweenAttributes=" + this.allowRemovingSpacesBetweenAttributes + ", keepClosingTags=" + this.keepClosingTags + ", keepComments=" + this.keepComments + ", keepHtmlAndHeadOpeningTags=" + this.keepHtmlAndHeadOpeningTags + ", keepInputTypeTextAttr=" + this.keepInputTypeTextAttr + ", keepSsiComments=" + this.keepSsiComments + ", minifyCss=" + this.minifyCss + ", minifyDoctype=" + this.minifyDoctype + ", minifyJs=" + this.minifyJs + ", preserveBraceTemplateSyntax=" + this.preserveBraceTemplateSyntax + ", preserveChevronPercentTemplateSyntax=" + this.preserveChevronPercentTemplateSyntax + ", removeBangs=" + this.removeBangs + ", removeProcessingInstructions=" + this.removeProcessingInstructions + ")";
        }
    }
}
Copilot PR review summary

Summary of PR #240: "feat: use lombok to generate java files"

  • Purpose:
    This PR migrates the Java code generation for the minify-html project from a custom JavaScript script to using Lombok, a popular Java annotation processor that reduces boilerplate code.
  • Key Changes:
    • Fixes a Maven version mismatch (refs Java versions aren't published #235).
    • Replaces the previous code generation (via a JS script) with Lombok annotations for the Configuration class, making the codebase more maintainable.
    • The builder pattern is retained and remains compatible, but field names are now camelCase (Java convention), which is a breaking change if users accessed fields directly.
    • The builder and general behavior are unchanged for typical usage.
    • Lombok-generated static methods and toString() are added for the builder and configuration.
    • No Lombok dependencies are included in the final compiled package.
    • Refactors OS/arch variable checks, adds null checks to prevent panics, and updates the Java-Rust FFI interface.
    • Updates Maven plugins and versions.
    • Removes the old JS code-gen script, replaces it with a bash script that relies on markers in Configuration.java and lib.rs.
    • Adds basic tests for the Java interface.
  • Request for Feedback:
    The PR author specifically asks whether the breaking change in field names (from snake_case to camelCase) is acceptable or if further adjustment is needed.

Code Review

Positive Aspects

  • Maintainability:
    Using Lombok removes a significant amount of boilerplate, especially for immutable value objects and builders.
  • Code Generation:
    The new bash script (update-java.sh) is clear and leverages source markers, which helps automate updates as the Rust config changes.
  • Testing:
    Basic tests are added, including not only a positive case but also null parameter validation.
  • Safety:
    The change from public final fields to Lombok-generated values and the move to camelCase fields encourages correct encapsulation and more idiomatic Java usage.
  • Maven Modernization:
    Dependency and plugin versions are updated, which is important for compatibility and security.

Points of Attention / Suggestions

  • Breaking Change (Field Names):
    • Changing the Configuration fields from snake_case to camelCase is a breaking change for anyone who accessed fields directly rather than via the builder. This is called out in the PR and is generally safe if the fields weren't meant for direct access. However, it should ideally be highlighted in release notes and the documentation.
    • If backwards compatibility is a major concern, consider keeping deprecated snake_case getters for one release cycle.
  • Encapsulation:
    • With Lombok’s @Value and @Builder, the fields become private and final, which is good. Ensure that the generated getters follow JavaBean conventions (they do, by default).
  • Native Interop:
    • The Rust FFI code was updated to call the new getter methods (e.g., getAllowNoncompliantUnquotedAttributeValues). This is correct and maintains the bridge.
  • Script Maintainability:
    • The bash script is more maintainable than the previous JS script but still requires developers to have a suitable shell environment. Consider documenting this dependency.
  • Tests:
    • The new Java tests are a great start. It could be beneficial to add more parameterized tests or edge cases in the future, especially as the configuration surface grows.

Conclusion

This PR is a strong improvement for maintainability and modernization of the Java API in minify-html. The one major breaking change (field naming) is well-documented and justified, but it should be communicated clearly to any library consumers. No significant issues found in the implementation—good use of Lombok, careful updates to interop, and improved project hygiene.

Open Todos

  • Error handling from rust code, if panic then the java code doesn't receive any returned function calls. The error can be thrown in java as exception from the JNI, but it requires changes that affects a larger scope than in this already big PR. I worked on it on a separate branch

@jogerj jogerj marked this pull request as draft June 27, 2025 20:12
@jogerj jogerj marked this pull request as ready for review June 28, 2025 01:10
jogerj added 3 commits June 28, 2025 12:32
the original code used 2-spaced indents and google java code seems to be most consistent to this.
- refactored some functions and finalized class

Signed-off-by: jogerj <[email protected]>
added tests to prevent this

Signed-off-by: jogerj <[email protected]>
@sgammon

This comment was marked as outdated.

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.

2 participants