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

Issue #11807: fixes exception on records and fields in RequireThis #12471

Merged
merged 1 commit into from Dec 3, 2022

Conversation

rnveach
Copy link
Member

@rnveach rnveach commented Nov 29, 2022

Issue #11807

result = true;
break;
final DetailAST parent = member.getParent();
if (parent.getType() != TokenTypes.RECORD_COMPONENT_DEF) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't fully sure, but I assume this RECORD_COMPONENT_DEF is not a final field by default?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

record components are implicitly final.

Copy link
Member Author

@rnveach rnveach Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: It seems this code isn't correct. I will try to fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code should be "fixed" now.

@rnveach
Copy link
Member Author

rnveach commented Nov 29, 2022

Regression: http://rveach.no-ip.org:81/checkstyle/regression/317/
No differences.

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Items

@nrmancuso
Copy link
Member

It would be a good idea to run check regression over the projects file that found the NPE: https://github.com/checkstyle/contribution/blob/master/checkstyle-tester/latest-projects-to-test-on.properties. The older project file has projects that have little or no record usage.

@rnveach rnveach force-pushed the issue_11807 branch 2 times, most recently from f5105f5 to 1196640 Compare December 2, 2022 15:52
Copy link
Member

@nrmancuso nrmancuso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Item:

@rnveach
Copy link
Member Author

rnveach commented Dec 2, 2022

Regression: http://rveach.no-ip.org:81/checkstyle/regression/318/

Only 1 difference, no new violation.

Copy link
Member

@nrmancuso nrmancuso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation is failing on input files.

Item:

@nrmancuso nrmancuso assigned romani and unassigned rnveach and nrmancuso Dec 2, 2022
@romani romani merged commit 847203f into checkstyle:master Dec 3, 2022
@rnveach rnveach deleted the issue_11807 branch December 3, 2022 16:30
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.

None yet

3 participants