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

Java: Add Java 9 module-related keywords #3628

Merged
merged 5 commits into from
Mar 31, 2018

Conversation

tfesenko
Copy link
Contributor

Registered the ten new Java 9 restricted keywords as keywords.
From Section 3.9 of http://cr.openjdk.java.net/~mr/jigsaw/spec/java-se-9-jls-diffs.pdf:

A further ten character sequences are restricted keywords: open, module,
requires, transitive, exports, opens, to, uses, provides, and with.

Keywords vs restricted keywords

Note that these are restricted keywords, it means that ideally they should be highlighted as keywords only when in module-info.java. From the Section 3.9 referenced above:

These
character sequences are tokenized as keywords solely where they appear as
terminals in the ModuleDeclaration and ModuleDirective productions (§7.7). They
are tokenized as identifiers everywhere else, for compatibility with programs
written prior to Java SE 9. There is one exception: immediately to the right of
the character sequence requires in the ModuleDirective production, the character
sequence transitive is tokenized as a keyword unless it is followed by a separator,
in which case it is tokenized as an identifier.

It makes this change similar to #3467 where var is not a keyword, but a reserved type name

Screenshot

This is what module-info.java looks like with this change:
screen shot 2018-03-26 at 4 40 51 pm

Registered the ten new Java 9 restricted keywords as keywords.
From Section 3.9 of http://cr.openjdk.java.net/~mr/jigsaw/spec/java-se-9-jls-diffs.pdf:
> A further ten character sequences are restricted keywords: open, module,
requires, transitive, exports, opens, to, uses, provides, and with.

Note that these are *restricted* keywords, it means that ideally they should be highlighted as keywords only when in `module-info.java`. From the Section 3.9 referenced above:
> These
character sequences are tokenized as keywords solely where they appear as
terminals in the ModuleDeclaration and ModuleDirective productions (§7.7). They
are tokenized as identifiers everywhere else, for compatibility with programs
written prior to Java SE 9. There is one exception: immediately to the right of
the character sequence requires in the ModuleDirective production, the character
sequence transitive is tokenized as a keyword unless it is followed by a separator,
in which case it is tokenized as an identifier.

It makes this change similar to ajaxorg#3467 where `var` is not a keyword, but a `reserved type name`
@codecov
Copy link

codecov bot commented Mar 26, 2018

Codecov Report

Merging #3628 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3628      +/-   ##
==========================================
- Coverage   64.86%   64.82%   -0.05%     
==========================================
  Files         471      471              
  Lines       47130    47303     +173     
  Branches     9001     9023      +22     
==========================================
+ Hits        30572    30663      +91     
- Misses      16558    16640      +82
Impacted Files Coverage Δ
lib/ace/mode/java_highlight_rules.js 100% <100%> (ø) ⬆️
lib/ace/layer/gutter.js 26.22% <0%> (-6.54%) ⬇️
lib/ace/lib/dom.js 60.5% <0%> (-2.18%) ⬇️
lib/ace/editor.js 77.21% <0%> (-1.56%) ⬇️
lib/ace/lib/event.js 41.66% <0%> (-0.18%) ⬇️
lib/ace/virtual_renderer.js 41.55% <0%> (-0.12%) ⬇️
lib/ace/lib/event_emitter.js 91.86% <0%> (-0.1%) ⬇️
lib/ace/layer/text_test.js 98.11% <0%> (+0.03%) ⬆️
lib/ace/keyboard/vim.js 83.6% <0%> (+0.25%) ⬆️
lib/ace/layer/marker.js 21.36% <0%> (+1.98%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ded791f...13aeade. Read the comment docs.

@nightwing
Copy link
Member

This looks good.

Note that these are restricted keywords, it means that ideally they should be highlighted as keywords only when in module-info.java. From the Section 3.9 referenced above:

Do you mean we need to still modify this code to not always highlight these keywords, or this is ok to merge as is?

If we need to modify the code are open|requires|transitive|exports|opens|to|uses|provides|with keywords only after module [\w.]+ {? and if yes can the module block contain nested {s?

@tfesenko
Copy link
Contributor Author

@nightwing wrote:

Do you mean we need to still modify this code to not always highlight these keywords, or this is ok to merge as is?

You can merge as is - it's a reasonable approximation with rare false positives (incorrectly highlighted open|requires when they are not specified inside a module declaration).

If we need to modify the code are open|requires|transitive|exports|opens|to|uses|provides|with keywords only after module [\w.]+ {?

This is a much better approach, it provides more accurate highlighting.
@nightwing, that would be great if you could do it OR point me how to do it.

and if yes can the module block contain nested {s?

No, a module block cannot contain nested {s
Copying the module declaration grammar from section 7.7 of Java 9 Language Specification for your reference:

ModuleDeclaration:
  {Annotation} [open] module Identifier {. Identifier}
  { {ModuleDirective} }

ModuleDirective:
  requires {RequiresModifier} ModuleName ;
  exports PackageName [to ModuleName {, ModuleName}] ; 
  opens PackageName [to ModuleName {, ModuleName}] ; 
  uses TypeName ;
  provides TypeName with TypeName {, TypeName} ;

RequiresModifier:
  (one of)
  transitive static

@nightwing
Copy link
Member

You can add rules with next for doing this.
Adding a rule

            {
                regex: "module(?=\\s*\\w)",
                token: "keyword",
                next: [{
                    regex: "{",
                    token: "paren.lparen",
                    next: [{
                        regex: "}",
                        token: "paren.rparen",
                        next: "start"
                    }, {
                        regex: "open|requires|transitive|exports|opens|to|uses|provides|with",
                        token: "keyword" 
                    }]
                }, {
                    token : "text",
                    regex : "\\s+"
                }, {
                    token : "identifier",
                    regex : "\\w+"
                }, {
                    token : "punctuation.operator",
                    regex : "."
                }, {
                    token : "text",
                    regex : "\\s+"
                }, {
                    regex: "", // exit if there is anything else
                    next: "start"
                }]
            }

before the one with keywordMapper

and adding this.normalizeRules(); at the end of the function, to allow using next: without explicitly naming the state, would be a good starting point.

@nightwing nightwing merged commit 77b932d into ajaxorg:master Mar 31, 2018
@nightwing
Copy link
Member

Thank you!

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

2 participants