Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@Singular, since 1.16.16: "warning: [cast] redundant cast to String" #1363

Open
tkruse opened this issue Apr 18, 2017 · 21 comments
Open

@Singular, since 1.16.16: "warning: [cast] redundant cast to String" #1363

tkruse opened this issue Apr 18, 2017 · 21 comments

Comments

@tkruse
Copy link

tkruse commented Apr 18, 2017

Since upgrading to 1.16.16, I get the following compiler warning:

 warning: [cast] redundant cast to String
@Builder
^

For a bean like:

@Data
@Builder
@AllArgsConstructor
public class MessageDTO {

    @Singular
    private Map<String, String> messages;

}
@tkruse
Copy link
Author

tkruse commented Apr 18, 2017

requires to enable javac compiler options to reproduce, obviously. Like

-Xlint:cast

@rspilker
Copy link
Collaborator

The problem with this is that we've recently introduced this "feature" to be able to compile using jdk9.

If upcoming jdk9 release no longer require this cast, we will remove it again.

@Akromangel
Copy link

Lombok v1.16.20 still has such warning.
Is this feature still required?

@Maaartinus
Copy link
Contributor

@rspilker IIRC the warning can be avoided using something like String s2 = (String) (Object) s; (possibly a temporary is needed; I can't recall, but in the end, javac and Eclipse were both happy). There may be some runtime cost for this hack.

@mcanalesmayo
Copy link

We are still running into this warning with lombok 1.18.16... any updates?

@ben-pearson
Copy link

Funny, that it's not been mentioned in nearly 3 years and I just ran into the same issue, and found this :(

@henrykuijpers
Copy link

And it's very funny that we have just run into this in november 2021 as well. :)

@rzwitserloot Would a good fix be adding the double cast? Or maybe adding an instanceof check before casting?

@henrykuijpers
Copy link

@rspilker Any thoughts on this?

@rspilker
Copy link
Collaborator

Hmm, so I guess that @SuppressWarnings("all") on the method isn't good enough. Is there a different suppression, like on a variable declaration? I'm hesitant about adding the double cast, I fear it might end up in the bytecode.

  1. Can someone test if there's a suppression possible?
  2. Can someone verify a double cast ends up in the bytecode?

@henrykuijpers
Copy link

I've tested the @SuppressWarnings("all") on class level, that didn't get rid of the warning. I also don't want to "enrich" our codebase with these warning suppressions.

I think, however, you mean adding that warning suppression in the code that Lombok generates? I suppose adding it to the very location where that cast takes place should work? And i think you can even add @SuppressWarnings("cast") instead of all.

I don't know if the double cast ends up in the generated bytecode, it could very well be the case it does, as the compiler probably can't optimize it away.

@rspilker
Copy link
Collaborator

@henrykuijpers, indeed, I was considering adding a suppression in the generated code. Could you run delombok, compile the code to make sure the warning still occurs, and the manually add the suppression to the variable declaration to make sure that the linter picks it up?

@henrykuijpers
Copy link

henrykuijpers commented Nov 24, 2021

@rspilker I just tried this example:

public class Testing {
    @Singular
    private final Map<String, String> parameters;
}

It delomboks to:

public class Testing {
    private final Map<String, String> parameters;

    @java.lang.SuppressWarnings("all")
    @javax.annotation.Generated("lombok")
    @lombok.Generated
    Testing(final Map<String, String> parameters) {
        this.parameters = parameters;
    }


    @java.lang.SuppressWarnings("all")
    @javax.annotation.Generated("lombok")
    @lombok.Generated
    public static class TestingBuilder {
        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        private java.util.ArrayList<String> parameters$key;
        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        private java.util.ArrayList<String> parameters$value;

        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        TestingBuilder() {
        }

        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        public Testing.TestingBuilder parameter(final String parameterKey, final String parameterValue) {
            if (this.parameters$key == null) {
                this.parameters$key = new java.util.ArrayList<String>();
                this.parameters$value = new java.util.ArrayList<String>();
            }
            this.parameters$key.add(parameterKey);
            this.parameters$value.add(parameterValue);
            return this;
        }

        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        public Testing.TestingBuilder parameters(final java.util.Map<? extends String, ? extends String> parameters) {
            if (parameters == null) {
                throw new java.lang.IllegalArgumentException("parameters cannot be null");
            }
            if (this.parameters$key == null) {
                this.parameters$key = new java.util.ArrayList<String>();
                this.parameters$value = new java.util.ArrayList<String>();
            }
            for (final java.util.Map.Entry<? extends String, ? extends String> $lombokEntry : parameters.entrySet()) {
                this.parameters$key.add($lombokEntry.getKey());
                this.parameters$value.add($lombokEntry.getValue());
            }
            return this;
        }

        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        public Testing.TestingBuilder clearParameters() {
            if (this.parameters$key != null) {
                this.parameters$key.clear();
                this.parameters$value.clear();
            }
            return this;
        }

        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        public Testing build() {
            java.util.Map<String, String> parameters;
            switch (this.parameters$key == null ? 0 : this.parameters$key.size()) {
                case 0:
                    parameters = java.util.Collections.emptyMap();
                    break;
                case 1:
                    parameters = java.util.Collections.singletonMap(this.parameters$key.get(0), this.parameters$value.get(0));
                    break;
                default:
                    parameters = new java.util.LinkedHashMap<String, String>(this.parameters$key.size() < 1073741824 ? 1 + this.parameters$key.size() + (this.parameters$key.size() - 3) / 3 : java.lang.Integer.MAX_VALUE);
                    for (int $i = 0; $i < this.parameters$key.size(); $i++) parameters.put(this.parameters$key.get($i), (String) this.parameters$value.get($i));
                    parameters = java.util.Collections.unmodifiableMap(parameters);
            }
            return new Testing(parameters);
        }

        @java.lang.Override
        @java.lang.SuppressWarnings("all")
        @javax.annotation.Generated("lombok")
        @lombok.Generated
        public java.lang.String toString() {
            return "Testing.TestingBuilder(parameters$key=" + this.parameters$key + ", parameters$value=" + this.parameters$value + ")";
        }
    }

    @java.lang.SuppressWarnings("all")
    @javax.annotation.Generated("lombok")
    @lombok.Generated
    public static Testing.TestingBuilder builder() {
        return new Testing.TestingBuilder();
    }
}

@henrykuijpers
Copy link

@rspilker If I change the @SuppressWarnings("all") to @SuppressWarnings({"all","cast"}) it compiles just fine, without the warning.

@henrykuijpers
Copy link

I would argue there could be some more wrong (but I'm not an expert in this), since the cast is used in the default case, but is not used in the singleton case:

            switch (this.parameters$key == null ? 0 : this.parameters$key.size()) {
                case 0:
                    parameters = java.util.Collections.emptyMap();
                    break;
                case 1:
                    parameters = java.util.Collections.singletonMap(this.parameters$key.get(0), this.parameters$value.get(0));
                    break;
                default:
                    parameters = new java.util.LinkedHashMap<String, String>(this.parameters$key.size() < 1073741824 ? 1 + this.parameters$key.size() + (this.parameters$key.size() - 3) / 3 : java.lang.Integer.MAX_VALUE);
                    for (int $i = 0; $i < this.parameters$key.size(); $i++) parameters.put(this.parameters$key.get($i), (String) this.parameters$value.get($i));
                    parameters = java.util.Collections.unmodifiableMap(parameters);
            }

Some things I could see to fix this:

  • Remove the cast (String), as it is indeed not needed. But, there is probably a reason it was added?
  • Add @SuppressWarnings("cast") final String $castedValue = (String)this.parameters$value.get($i); and then use $castedValue as the second argument to put

If the cast really cannot be removed, then I suggest also changing the case 1, since then that cast should also be there?

I also thought @SuppressWarnings("all") would literally suppress all warnings, apparently cast doesn't fall in that category though? Or is it the overload of @SuppressWarnings declarations in this class, that somehow confuses the compiler?

@henrykuijpers
Copy link

Interesting observations here: antlr/antlr3#186 (comment)

@rspilker
Copy link
Collaborator

Regarding the case 1, that's different because the type inference engine and type verification is handle at a different place. The type parameters of the singletonMap are derived from the parameters, where in the put call, the type parameters are specified when the local variable parameters is declared.

For some reason, the assignment of the singletonMap is deemed ok.

Regarding the "all", I think people should put pressure on the javac lint team to allow for blanket suppression of warnings, especially in (partly) generated code.

@henrykuijpers
Copy link

@rspilker Do you have a suggestion for a way to fix this? For example, by putting @SuppressWarnings({"all","cast"})?

@remal
Copy link

remal commented Sep 14, 2023

@rspilker, we use -Werror javac option. So, our build fails for the scenario mentioned in this ticket. Is there a way to fix it?

Java version: 17
Lombok version: 1.18.28

@alexec
Copy link

alexec commented Dec 23, 2023

I get this issue with this when using with -Werror.

  @Singular private Map<String,?> properties;
/Users/acollins8/api-gov/sacf4j/sacf4j-api/src/main/java/com/intuit/sacf4j/Config.java:13
java: redundant cast to java.lang.Object

You can work around as follows:

  @Singular private List<Map.Entry<String, Object>> properties;

@rzwitserloot
Copy link
Collaborator

We need this spelled out for us. We need the following:

  • A fully self contained snippet of code that represents 'compile this with lombok on the classpath'.
  • The code that lombok currently produces (the output of java -jar lombok.jar delombok -p ThatFile.java)
  • The code that you would want it to write which avoids this error.
  • The command you'd invoke to make the code from the second step (the delomboked code) produce the warning or error you'd like not to see.

@remal
Copy link

remal commented Jan 13, 2024

A reproduction (Java 17):

import java.util.Map;
import lombok.Builder;
import lombok.Singular;
import lombok.Value;

@Value
@Builder
class Test {

    @Singular
    private Map<String, String> messages;

}
javac --class-path lombok.jar -Xlint:all Test.java output
Test.java:7: warning: [cast] redundant cast to String
@Builder
^
1 warning
java -jar lombok.jar delombok -p Test.java output
import java.util.Map;

final class Test {
    private final Map<String, String> messages;

    @java.lang.SuppressWarnings("all")
    Test(final Map<String, String> messages) {
        this.messages = messages;
    }


    @java.lang.SuppressWarnings("all")
    public static class TestBuilder {
        @java.lang.SuppressWarnings("all")
        private java.util.ArrayList<String> messages$key;
        @java.lang.SuppressWarnings("all")
        private java.util.ArrayList<String> messages$value;

        @java.lang.SuppressWarnings("all")
        TestBuilder() {
        }

        @java.lang.SuppressWarnings("all")
        public Test.TestBuilder message(final String messageKey, final String messageValue) {
            if (this.messages$key == null) {
                this.messages$key = new java.util.ArrayList<String>();
                this.messages$value = new java.util.ArrayList<String>();
            }
            this.messages$key.add(messageKey);
            this.messages$value.add(messageValue);
            return this;
        }

        @java.lang.SuppressWarnings("all")
        public Test.TestBuilder messages(final java.util.Map<? extends String, ? extends String> messages) {
            if (messages == null) {
                throw new java.lang.NullPointerException("messages cannot be null");
            }
            if (this.messages$key == null) {
                this.messages$key = new java.util.ArrayList<String>();
                this.messages$value = new java.util.ArrayList<String>();
            }
            for (final java.util.Map.Entry<? extends String, ? extends String> $lombokEntry : messages.entrySet()) {
                this.messages$key.add($lombokEntry.getKey());
                this.messages$value.add($lombokEntry.getValue());
            }
            return this;
        }

        @java.lang.SuppressWarnings("all")
        public Test.TestBuilder clearMessages() {
            if (this.messages$key != null) {
                this.messages$key.clear();
                this.messages$value.clear();
            }
            return this;
        }

        @java.lang.SuppressWarnings("all")
        public Test build() {
            java.util.Map<String, String> messages;
            switch (this.messages$key == null ? 0 : this.messages$key.size()) {
            case 0:
                messages = java.util.Collections.emptyMap();
                break;
            case 1:
                messages = java.util.Collections.singletonMap(this.messages$key.get(0), this.messages$value.get(0));
                break;
            default:
                messages = new java.util.LinkedHashMap<String, String>(this.messages$key.size() < 1073741824 ? 1 + this.messages$key.size() + (this.messages$key.size() - 3) / 3 : java.lang.Integer.MAX_VALUE);
                for (int $i = 0; $i < this.messages$key.size(); $i++) messages.put(this.messages$key.get($i), (String) this.messages$value.get($i));
                messages = java.util.Collections.unmodifiableMap(messages);
            }
            return new Test(messages);
        }

        @java.lang.Override
        @java.lang.SuppressWarnings("all")
        public java.lang.String toString() {
            return "Test.TestBuilder(messages$key=" + this.messages$key + ", messages$value=" + this.messages$value + ")";
        }
    }

    @java.lang.SuppressWarnings("all")
    public static Test.TestBuilder builder() {
        return new Test.TestBuilder();
    }

    @java.lang.SuppressWarnings("all")
    public Map<String, String> getMessages() {
        return this.messages;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public boolean equals(final java.lang.Object o) {
        if (o == this) return true;
        if (!(o instanceof Test)) return false;
        final Test other = (Test) o;
        final java.lang.Object this$messages = this.getMessages();
        final java.lang.Object other$messages = other.getMessages();
        if (this$messages == null ? other$messages != null : !this$messages.equals(other$messages)) return false;
        return true;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final java.lang.Object $messages = this.getMessages();
        result = result * PRIME + ($messages == null ? 43 : $messages.hashCode());
        return result;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public java.lang.String toString() {
        return "Test(messages=" + this.getMessages() + ")";
    }
}
The command you'd invoke to make the code from the second step (the delomboked code) produce the warning: javac --class-path lombok.jar -Xlint:all delomboked.java
delomboked.java:71: warning: [cast] redundant cast to String
                for (int $i = 0; $i < this.messages$key.size(); $i++) messages.put(this.messages$key.get($i), (String) this.messages$value.get($i));
                                                                                                              ^
1 warning
The code that you would want it to write which avoids this error
import java.util.Map;

final class Test {
    private final Map<String, String> messages;

    @java.lang.SuppressWarnings("all")
    Test(final Map<String, String> messages) {
        this.messages = messages;
    }


    @java.lang.SuppressWarnings("all")
    public static class TestBuilder {
        @java.lang.SuppressWarnings("all")
        private java.util.ArrayList<String> messages$key;
        @java.lang.SuppressWarnings("all")
        private java.util.ArrayList<String> messages$value;

        @java.lang.SuppressWarnings("all")
        TestBuilder() {
        }

        @java.lang.SuppressWarnings("all")
        public Test.TestBuilder message(final String messageKey, final String messageValue) {
            if (this.messages$key == null) {
                this.messages$key = new java.util.ArrayList<String>();
                this.messages$value = new java.util.ArrayList<String>();
            }
            this.messages$key.add(messageKey);
            this.messages$value.add(messageValue);
            return this;
        }

        @java.lang.SuppressWarnings("all")
        public Test.TestBuilder messages(final java.util.Map<? extends String, ? extends String> messages) {
            if (messages == null) {
                throw new java.lang.NullPointerException("messages cannot be null");
            }
            if (this.messages$key == null) {
                this.messages$key = new java.util.ArrayList<String>();
                this.messages$value = new java.util.ArrayList<String>();
            }
            for (final java.util.Map.Entry<? extends String, ? extends String> $lombokEntry : messages.entrySet()) {
                this.messages$key.add($lombokEntry.getKey());
                this.messages$value.add($lombokEntry.getValue());
            }
            return this;
        }

        @java.lang.SuppressWarnings("all")
        public Test.TestBuilder clearMessages() {
            if (this.messages$key != null) {
                this.messages$key.clear();
                this.messages$value.clear();
            }
            return this;
        }

        @java.lang.SuppressWarnings({"all", "cast"}) // <-- THE ONLY DIFFERENCE IS HERE - "cast" ADDITION
        public Test build() {
            java.util.Map<String, String> messages;
            switch (this.messages$key == null ? 0 : this.messages$key.size()) {
            case 0:
                messages = java.util.Collections.emptyMap();
                break;
            case 1:
                messages = java.util.Collections.singletonMap(this.messages$key.get(0), this.messages$value.get(0));
                break;
            default:
                messages = new java.util.LinkedHashMap<String, String>(this.messages$key.size() < 1073741824 ? 1 + this.messages$key.size() + (this.messages$key.size() - 3) / 3 : java.lang.Integer.MAX_VALUE);
                for (int $i = 0; $i < this.messages$key.size(); $i++) messages.put(this.messages$key.get($i), (String) this.messages$value.get($i));
                messages = java.util.Collections.unmodifiableMap(messages);
            }
            return new Test(messages);
        }

        @java.lang.Override
        @java.lang.SuppressWarnings("all")
        public java.lang.String toString() {
            return "Test.TestBuilder(messages$key=" + this.messages$key + ", messages$value=" + this.messages$value + ")";
        }
    }

    @java.lang.SuppressWarnings("all")
    public static Test.TestBuilder builder() {
        return new Test.TestBuilder();
    }

    @java.lang.SuppressWarnings("all")
    public Map<String, String> getMessages() {
        return this.messages;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public boolean equals(final java.lang.Object o) {
        if (o == this) return true;
        if (!(o instanceof Test)) return false;
        final Test other = (Test) o;
        final java.lang.Object this$messages = this.getMessages();
        final java.lang.Object other$messages = other.getMessages();
        if (this$messages == null ? other$messages != null : !this$messages.equals(other$messages)) return false;
        return true;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final java.lang.Object $messages = this.getMessages();
        result = result * PRIME + ($messages == null ? 43 : $messages.hashCode());
        return result;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public java.lang.String toString() {
        return "Test(messages=" + this.getMessages() + ")";
    }
}

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

No branches or pull requests

10 participants