Skip to content

Issue #8023: Add input file for Java14 records #8290

Merged
pbludov merged 1 commit intocheckstyle:masterfrom
nrmancuso:records-input
Jun 4, 2020
Merged

Issue #8023: Add input file for Java14 records #8290
pbludov merged 1 commit intocheckstyle:masterfrom
nrmancuso:records-input

Conversation

@nrmancuso
Copy link
Copy Markdown
Contributor

@nrmancuso nrmancuso commented Jun 1, 2020

Issue #8023: Add records input

Examples taken from:
https://blogs.oracle.com/javamagazine/records-come-to-java
https://alidg.me/blog/2020/2/9/java14-records-in-depth
https://openjdk.java.net/jeps/359

compilation is checked by:

checkstyle/.travis.yml

Lines 289 to 293 in 15ba4cc

# OpenJDK14 compile input files with jdk14 specific syntax
- jdk: openjdk14
env:
- DESC="compile input files with jdk14 specific syntax"
- CMD="./.ci/travis/travis.sh javac14"

@romani
Copy link
Copy Markdown
Member

romani commented Jun 1, 2020

@nmancus1 , please keep CI happy all the time - https://travis-ci.org/github/checkstyle/checkstyle/jobs/693436338#L636

@romani
Copy link
Copy Markdown
Member

romani commented Jun 1, 2020

@nmancus1 , did you find any interesting examples in openjdk project ?
I stumbled a lot on records in openjdk14.

@nrmancuso
Copy link
Copy Markdown
Contributor Author

please keep CI happy all the time

What should I do in this case? Isn't this failing since we can't parse this file yet?

did you find any interesting examples in openjdk project

Yes, should I provide more examples?

@romani
Copy link
Copy Markdown
Member

romani commented Jun 1, 2020

from travis:

[INFO]      [echo] Checkstyle started (checkstyle_resources_checks.xml): 01/06/2020 01:11:03 PM
[INFO] [checkstyle] Running Checkstyle  on 1853 files
[ERROR] [checkstyle] [ERROR] /home/travis/build/checkstyle/checkstyle/src/test/resources-noncompilable

Checkstyle should not try to parse such files at all, we do not support non-compilable files at all, please investigate our config do not filter out such files.
all is here - https://github.com/checkstyle/checkstyle/blob/master/config/ant-phase-verify.xml

should I provide more examples?

if examples are different in some way - yes,
if no interesting cases - just make it clear in pr description that nothing special found.

@nrmancuso
Copy link
Copy Markdown
Contributor Author

Checkstyle should not try to parse such files at all, we do not support non-compilable files at all

We are still checking them with config checkstyle_resources_checks.xml; OuterTypeFilenameCheck is causing the issue. I have explicitly added InputJava14Records.java to the excludes list.

@esilkensen
Copy link
Copy Markdown
Member

Looks good to me once CI is green!

@nrmancuso nrmancuso changed the title Issue #8023: Add records input Issue #8023: Add input file for Java14 records Jun 1, 2020
@esilkensen esilkensen requested a review from pbludov June 2, 2020 14:13
@esilkensen esilkensen assigned pbludov and unassigned esilkensen Jun 2, 2020
@nrmancuso nrmancuso marked this pull request as draft June 2, 2020 18:49
@nrmancuso nrmancuso marked this pull request as ready for review June 2, 2020 22:15
@pbludov pbludov merged commit 41081a7 into checkstyle:master Jun 4, 2020
@romani
Copy link
Copy Markdown
Member

romani commented Jun 9, 2020

@pbludov , @esilkensen , please do not allow commit message to start from issue, if it does not fix issue. in release notes it will looks like we provided support for this issue, but do not, and we just did some preparation fix for this issue. in such case you can do minor: .... (#8023)

@pbludov
Copy link
Copy Markdown
Member

pbludov commented Jun 10, 2020

My bad. I didn't even look at the commit description. I'll keep it in mind for the future.

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.

4 participants