Issue #8023: Update grammar for initial record support by nrmancuso · Pull Request #8293 · checkstyle/checkstyle

nrmancuso · 2020-06-01T19:37:14Z

Issue #8023: Update grammar for initial record support

TO DO:

Unit tests (including ASTRegressionTest after Issue #8023: Add input file for Java14 records #8290 is merged)
Modify grammar to avoid "record" in variable declaration:

checkstyle/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/grammar/java14/InputJava14Records.java

Lines 103 to 104 in 2c33bb0

LogRecord record;

record = new LogRecord(Level.ALL, "abc");

Modify grammar to handle no arg record constructor:

checkstyle/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/grammar/java14/InputJava14Records.java

Lines 46 to 49 in 2c33bb0

    
           public Thing { 
        
               Objects.requireNonNull(name1); 
        
               Objects.requireNonNull(name2); 
        
           }

Mentors, please feel free to edit this post to add more items in TODO, or reply and I will add them.

nrmancuso · 2020-06-01T20:02:58Z

AST Regression Reports 6/15 and 6/17
https://nmancus1.github.io/initial-record-grammar_check_diff_reports_2020_06_15/AST_diff_6-15_checkstyle_ant
https://nmancus1.github.io/initial-record-grammar_check_diff_reports_2020_06_15/AST_diff_6-17_jdk14

Coverage Reports
https://nmancus1.github.io/site_initial-records-grammar/jacoco
https://nmancus1.github.io/site_master/jacoco

...sources-noncompilable/com/puppycrawl/tools/checkstyle/grammar/java14/InputJava14Records.java

src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g

nrmancuso · 2020-06-11T15:23:32Z

I am reducing the minimum coverage for GeneratedJavaRecognizer.java from 0.73 to 0.72. The reason for this is because the code generated by the new grammar rules I have introduced have adequate coverage. There are only two rules that don't have 100% coverage. These are:

recordComponents

This amount of coverage is typical for rules that use t:typeSpec[false] , like enumConstantField.

recordDeclaration

Other than the default option, the only branch not covered is the false comparison result of the ternary statment on line 818, and the same branch in classDefinition (which is very similar to recordDeclaration) is also not covered:

Below is the coverage for all of the rules I've added:

`

rnveach · 2020-06-11T20:54:01Z

@nmancus1 FYI on how to hit the harder to reach exceptions:

checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/grammar/AstRegressionTest.java

Line 201 in dabe0ff

public void testImpossibleExceptions() throws Exception {

nrmancuso · 2020-06-11T21:26:38Z

Execution time testing on guava with google_checks.xml (I've omitted the other time outputs and just kept the total):

➜ checkstyle-tester git:(master) ✗ for run in {1..20}; do groovy diff.groovy --localGitRepo /home/nick/IdeaProjects/checkstyle --patchBranch master --patchConfig ../../google_checks.xml --listOfProjects projects-to-test-on.properties --mode single |& grep 'Total time:'; done

[INFO] Total time:  42.857 s
[INFO] Total time:  43.172 s
[INFO] Total time:  42.233 s
[INFO] Total time:  43.812 s
[INFO] Total time:  42.149 s
[INFO] Total time:  44.875 s
[INFO] Total time:  46.218 s
[INFO] Total time:  43.659 s
[INFO] Total time:  45.105 s
[INFO] Total time:  42.515 s
[INFO] Total time:  43.349 s
[INFO] Total time:  45.786 s
[INFO] Total time:  45.326 s
[INFO] Total time:  43.102 s
[INFO] Total time:  43.368 s
[INFO] Total time:  43.545 s
[INFO] Total time:  44.725 s
[INFO] Total time:  44.314 s
[INFO] Total time:  46.607 s
[INFO] Total time:  43.914 s

➜ checkstyle-tester git:(master) ✗ for run in {1..20}; do groovy diff.groovy --localGitRepo /home/nick/IdeaProjects/checkstyle --patchBranch initial-record-grammar --patchConfig ../../google_checks.xml --listOfProjects projects-to-test-on.properties --mode single |& grep 'Total time:'; done

[INFO] Total time:  44.310 s
[INFO] Total time:  43.649 s
[INFO] Total time:  42.931 s
[INFO] Total time:  43.118 s
[INFO] Total time:  44.582 s
[INFO] Total time:  44.762 s
[INFO] Total time:  43.479 s
[INFO] Total time:  43.130 s
[INFO] Total time:  43.646 s
[INFO] Total time:  42.599 s
[INFO] Total time:  42.074 s
[INFO] Total time:  42.578 s
[INFO] Total time:  42.702 s
[INFO] Total time:  42.881 s
[INFO] Total time:  42.860 s
[INFO] Total time:  44.799 s
[INFO] Total time:  45.955 s
[INFO] Total time:  43.048 s
[INFO] Total time:  43.801 s
[INFO] Total time:  43.279 s

Which gives an average time of 44.031s for master, and 43.509s for initial-record-grammar.

nrmancuso · 2020-06-12T12:17:26Z

FYI on how to hit the harder to reach exceptions

@rnveach
Unless I'm missing something, these tests are for the lexer only.

pom.xml

nrmancuso · 2020-06-12T15:47:14Z

With these grammar changes, there has been some additional nondeterminism detected:

Sans line numbers:

Aside from where nondeterminism existed previously, these are the rules in which it has been introduced:

java.g:443: warning:nondeterminism upon
java.g:443:     k==1:DOT
java.g:443:     k==2:"record"
java.g:443:     between alt 1 and exit branch of block

java.g:951: warning:nondeterminism between alts 1 and 2 of block upon
java.g:951:     k==1:AT
java.g:951:     k==2:RBRACK,IDENT,"record"

java.g:761:9: warning:nondeterminism between alts 1 and 2 of block upon
java.g:761:9:     k==1:AT,"record"
java.g:761:9:     k==2:IDENT,"record"

java.g:1045:31: warning:nondeterminism between alts 1 and 2 of block upon
java.g:1045:31:     k==1:IDENT,"record"
java.g:1045:31:     k==2:LBRACK,DOT,AT

These lines correspond to the identifierStar, declaratorBrackets, enumConstantField, and parameterDeclaration rules, respectively.

rnveach · 2020-06-24T21:37:26Z

Can someone post the JLS link for records and how they specify the language?

Also I notice this isn't full record support. Is there another issue defined that will house the full record support?

src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g

nrmancuso · 2020-06-24T22:06:52Z

Can someone post the JLS link for records and how they specify the language?

See #8023 (comment)

rnveach

Only minor things.

src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g

rnveach · 2020-07-02T13:38:18Z

I think this is ok to merge when CI passes. @romani

romani

minors:

src/main/java/com/puppycrawl/tools/checkstyle/api/TokenTypes.java

src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g

romani

item:

src/main/java/com/puppycrawl/tools/checkstyle/api/TokenTypes.java

romani · 2020-07-08T14:42:42Z

there are conflicts, please rebase and prepare for merge.

…style#8023)

nrmancuso force-pushed the initial-record-grammar branch 10 times, most recently from d77d8dc to 6e12ca7 Compare June 6, 2020 12:20

esilkensen reviewed Jun 9, 2020

View reviewed changes

...sources-noncompilable/com/puppycrawl/tools/checkstyle/grammar/java14/InputJava14Records.java Show resolved Hide resolved

src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g Outdated Show resolved Hide resolved

nrmancuso mentioned this pull request Jun 10, 2020

Java Grammar: need to introduce "id" rule to solve keyword as identifier problem #8308

Closed

nrmancuso force-pushed the initial-record-grammar branch from 6e12ca7 to 114bc18 Compare June 10, 2020 21:27

nrmancuso force-pushed the initial-record-grammar branch 2 times, most recently from 0449997 to 4ed9792 Compare June 11, 2020 17:35

nrmancuso force-pushed the initial-record-grammar branch from 4ed9792 to af2bd54 Compare June 12, 2020 14:37

esilkensen reviewed Jun 12, 2020

View reviewed changes

pom.xml Show resolved Hide resolved

nrmancuso force-pushed the initial-record-grammar branch from af2bd54 to 4ea08c6 Compare June 12, 2020 15:16

nrmancuso marked this pull request as ready for review June 13, 2020 00:57

nrmancuso force-pushed the initial-record-grammar branch 3 times, most recently from a6ae552 to 20015f7 Compare June 15, 2020 14:46

nrmancuso marked this pull request as draft June 15, 2020 14:47

nrmancuso marked this pull request as ready for review June 16, 2020 12:46

esilkensen reviewed Jun 24, 2020

View reviewed changes

src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g Show resolved Hide resolved

nrmancuso force-pushed the initial-record-grammar branch 3 times, most recently from 1b9932d to 1d7ea67 Compare June 26, 2020 04:11

rnveach reviewed Jul 1, 2020

View reviewed changes

esilkensen mentioned this pull request Jul 2, 2020

Issue #8354: MemberName check throws NPE when parsing enhanced instanceof #8392

Closed

nrmancuso force-pushed the initial-record-grammar branch 2 times, most recently from d3c25d6 to d9dd24d Compare July 2, 2020 13:26

rnveach approved these changes Jul 2, 2020

View reviewed changes

rnveach assigned romani and unassigned rnveach Jul 2, 2020

romani requested changes Jul 2, 2020

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/api/TokenTypes.java Outdated Show resolved Hide resolved

src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g Outdated Show resolved Hide resolved

nrmancuso force-pushed the initial-record-grammar branch from d9dd24d to bef4f03 Compare July 2, 2020 14:29

nrmancuso requested a review from romani July 7, 2020 02:49

romani requested changes Jul 7, 2020

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/api/TokenTypes.java Outdated Show resolved Hide resolved

nrmancuso force-pushed the initial-record-grammar branch from bef4f03 to 7cc4ed5 Compare July 7, 2020 15:18

romani approved these changes Jul 8, 2020

View reviewed changes

nrmancuso force-pushed the initial-record-grammar branch from 7cc4ed5 to 73deaaa Compare July 8, 2020 15:09

nrmancuso added 2 commits July 8, 2020 11:10

Issue checkstyle#8023: Update grammar for initial record support

d7bc777

minor: lower coverage minimum for GeneratedJavaRecognizer.java (check…

153d969

…style#8023)

nrmancuso force-pushed the initial-record-grammar branch from 73deaaa to 153d969 Compare July 8, 2020 15:11

This was referenced Jul 8, 2020

Issue #8345: Add TokenUtil.isTypeDeclaration method #8421

Merged

Issue #8267: Implement full records support #8422

Merged

pbludov approved these changes Jul 9, 2020

View reviewed changes

pbludov merged commit 84f33ea into checkstyle:master Jul 9, 2020

pbludov unassigned romani Jul 9, 2020

	public Thing {
	Objects.requireNonNull(name1);
	Objects.requireNonNull(name2);
	}

	LogRecord record;
	record = new LogRecord(Level.ALL, "abc");

Uh oh!

Conversation

nrmancuso commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TO DO:

Uh oh!

nrmancuso commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nrmancuso commented Jun 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rnveach commented Jun 11, 2020

Uh oh!

nrmancuso commented Jun 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrmancuso commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nrmancuso commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rnveach commented Jun 24, 2020

Uh oh!

Uh oh!

nrmancuso commented Jun 24, 2020

Uh oh!

rnveach left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rnveach commented Jul 2, 2020

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

romani commented Jul 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nrmancuso commented Jun 1, 2020 •

edited

Loading

nrmancuso commented Jun 1, 2020 •

edited

Loading

nrmancuso commented Jun 11, 2020 •

edited

Loading

nrmancuso commented Jun 11, 2020 •

edited

Loading

nrmancuso commented Jun 12, 2020 •

edited

Loading

nrmancuso commented Jun 12, 2020 •

edited

Loading