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

Remove unused variable in RecordBuilder #11

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

TedCassirer
Copy link

Minor issue
There's an unused variable r in the builder if the record only have 1 field.
Spotbugs flagged the generated source in my project because of it. This should fix the issue.

Example:

@RecordBuilder
public record MyRecord(long id) implements MyRecordBuilder.With {}

Generates ->

@Generated("io.soabase.recordbuilder.core.RecordBuilder")
public class MyRecordBuilder {
        private long id;

...
....
        /**
         * Return a new instance of {@code MyRecord} with a new value for {@code id}
         */
        @Generated("io.soabase.recordbuilder.core.RecordBuilder")
        default MyRecord withId(long id) {
            var r = (MyRecord)(Object)this;  //<---UNUSED
            return new MyRecord(id);
        }
}

I could just exclude the generated sources from spotbugs if you consider this a non bug

@Randgalt
Copy link
Owner

Thanks again.

@Randgalt Randgalt merged commit 7e8ddbd into Randgalt:master Nov 25, 2020
@TedCassirer
Copy link
Author

Ah, I commited a misstake. Noticed it immediately after creating the PR. Sorry!
I'll push the correct fix in a min

@Randgalt
Copy link
Owner

lol - I should test these before I merge

@TedCassirer
Copy link
Author

Disregard that. I just messed up updating my dependencies in my project causing me to build with the incorrect fix. It works as intended.

        /**
         * Return a new instance of {@code MyRecord} with a new value for {@code id}
         */
        @Generated("io.soabase.recordbuilder.core.RecordBuilder")
        default MyRecord withId(long id) {
            return new MyRecord(id);
        }

@Randgalt
Copy link
Owner

I apologize, Github isn't giving you credit for these PRs due to: isaacs/github#1368

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