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

Add support for the Java 8 syntax #10

Closed
pellaton opened this issue Oct 4, 2013 · 77 comments
Closed

Add support for the Java 8 syntax #10

pellaton opened this issue Oct 4, 2013 · 77 comments

Comments

@pellaton
Copy link

pellaton commented Oct 4, 2013

The upcoming Java 8 has a new language syntax that Checkstyle needs to support. The developer preview is quite stable and language syntax changes are not expected.

Besides that, the new language features might be an opportunity for new checks.

From RomanIvanov: please do "1 question" survey about Java8 support, without registration - https://www.surveymonkey.com/s/MNFHYSJ

@romani
Copy link
Member

romani commented Oct 4, 2013

sure, everyone are welcome with pull request.

For now we, our tiny team, do transitions from SourceForge to Github to analyse new priorities and set of tasks to fix in current implementation.

You are right, Java8 syntax will be a challenge for all existing Checks, so please do not expect java8 support in Checkstyle to appear quickly, sorry, but it is definitely a priority task.

@Philipp91
Copy link

This seems to be a pretty important issue to me. Java 8 will be released in a little more than two months and many people will make use of lambdas. So it would be very unfortunate if you had to turn off Checkstyle only because it does not read lambdas. It's not so much about checking the correct use of lambdas (yet), the only urgent issue is not to fail when parsing a file with lambdas and maybe to check the code inside the lambda expression normally.

@nipafx
Copy link

nipafx commented Jan 29, 2014

I wanna second this. I just started to use CheckStyle and don't want to give it up already.(Awesome tool - thank you very much!)
Unfortunately the Expecting-EOF-Exception the TreeWalker throws after each Lambda makes using it impossible. It would suffice if by the time Java 8 is officially released, CheckStyle would simply ignore Lambdas and check the rest of the file as usual.

@daniilyar
Copy link
Contributor

+1. Java 8 support would be a great feature.

@dlopatin
Copy link
Contributor

dlopatin commented Feb 7, 2014

We are planing to move our project to java 8 in one year, so this would be great to add its support to Check Style project.

@romani
Copy link
Member

romani commented Feb 9, 2014

2 all, please do "1 question" survey about Java8 support, without registration - https://www.surveymonkey.com/s/MNFHYSJ

@romani
Copy link
Member

romani commented Feb 12, 2014

We got connection with ANTLR team https://groups.google.com/forum/#!topic/antlr-discussion/AQOawdwDXFA , thank a lot to Terence Parr.

@eskatos
Copy link

eskatos commented Mar 18, 2014

Using 5.7 every file that contains a lambda expression get the expecting EOF, found '}' error and is not checked further.

I understand that supporting the whole Java 8 syntax and coming up with checks for new idioms is a big task.

Like @nicolaiparlog suggested, shouldn't it be possible to make a first step that simply gracefully ignore Java 8 peculiarities so we all get a checkstyle version that is usable with Java 8 sooner, even if it is not full-featured?

I have the gut feeling that that's what you're after with Java 8 ANTLR support. If this is the case, please tell us :-)

Cheers

@romani
Copy link
Member

romani commented Mar 20, 2014

We are approved organization at GSoC 2014, so Java8 support will be in few month. If we found simple way to ignore lambda syntax easily we will do it sooner than whole Java8 support.

@cbanack
Copy link

cbanack commented Mar 20, 2014

@romani: Thanks for the update!

If you guys can find a way to make Checkstyle ignore lambda expressions (i.e. no error), I think that would be a really useful first step. Once that problem is solved, developers won't have to choose between using Checkstyle or using lambda expressions. At that point, it's no big deal if it takes a long time to do the rest of the Java 8 support.

@eskatos
Copy link

eskatos commented Mar 20, 2014

Default methods cause the very same issue: expecting EOF, found '}' and whole file unchecked.

@Zlika
Copy link

Zlika commented Apr 12, 2014

Hi there!
Still no workaround for this issue? I don't want to choose between using Java 8 and using checkstyle! :-(

@ghost
Copy link

ghost commented Apr 22, 2014

@Zlika I here ya. I'm working on project right now with the best opportunity to utilize java 8's features like the default interface methods for instance and I don't know what to do. Everyone on my team is relatively new to team projects with code and just getting rid of checkstyle just seems like such a bad idea!

I do hope that they update checkstyle soon to support java 8.

@romani
Copy link
Member

romani commented Apr 22, 2014

Checkstyle will be updated to java8 probably in next month to ignore lambda at all, final support for java8 will be ready at the end of August of this year.

@sabaka
Copy link
Contributor

sabaka commented May 4, 2014

Hi!
I work on this problem. Could you please share java 8 projects with me to make me able to do testing on real sources? (if you have open source of course)

@nipafx
Copy link

nipafx commented May 5, 2014

Hi!

Yeah, sure. This is a very small project in its earliest stages but it
uses lambdas in its tests:

https://github.com/nicolaiparlog/codefx/tree/feature/nesting

so long ... Nicolai

Am 04.05.2014 10:34, schrieb Ilia Dubinin:

Hi!
I work on this problem. Could you please share java 8 projects with me to make me able to do testing on real sources? (if you have open source of course)


Reply to this email directly or view it on GitHub:
#10 (comment)

@sabaka
Copy link
Contributor

sabaka commented May 8, 2014

Thanks a lot!

@T3rm1
Copy link

T3rm1 commented May 13, 2014

How long will it take? Any ETA? :)

@sabaka
Copy link
Contributor

sabaka commented May 13, 2014

Hi!
It is my GSOC issue. It has to be done till 18th of August, but I'm going to finish in July.

Also, after discuss with team, we decide, that implementation of ignoring option for Java 8 features is painful, and may break grammar. Unfortunately community has to wait for full implementation of Java 8 features.

@mritun
Copy link

mritun commented Jun 11, 2014

+1

1 similar comment
@wendigo
Copy link

wendigo commented Jun 25, 2014

👍

@gredler
Copy link

gredler commented Jun 25, 2014

Sabaka: In terms of sample code, you may want to consider testing against the OpenJDK 8 codebase, which already has many examples of default methods on interfaces, lambdas, etc.

For example, these tests use lambdas extensively:

http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/test/java/util/stream/test/org/openjdk/tests/java/util/stream

@tnn
Copy link

tnn commented Jun 29, 2014

+1 This is blocking us also.

@satoshi-kimura
Copy link

👍

@zaskar9
Copy link

zaskar9 commented Jul 4, 2014

+1 Support for Java 8 (in particular Lambda expressions) would be awesome!

@devacto
Copy link

devacto commented Jul 9, 2014

+1 for Java 8 support. Thanks!

@LorenzSchumann
Copy link

+1 for Java 8 Support.
What is the state of your development, sabaka?

@sabaka
Copy link
Contributor

sabaka commented Jul 9, 2014

Hi!
Now I implemented support of default methods, method references (::) and almost finished with lambdas.

@khozzy
Copy link

khozzy commented Jul 10, 2014

+1 We also need it. Thanks in advance.

@zapodot
Copy link

zapodot commented Aug 20, 2014

@henrik242 the pull request #226 has not been merged into upstream yet. You may want to try with your own build of @sabaka 's clone at https://github.com/sabaka/checkstyle

@henrik242
Copy link

@zapodot Ahh, of course. Thanks :)

@cbanack
Copy link

cbanack commented Aug 21, 2014

When I try to build @sabaka 's clone (gsoc branch) with mvn -Pdistro clean package, I get exactly the same build error that @brutall got a few comments back, i.e. this one:

https://gist.github.com/brutall/f6594b1986011ace76b9

Is there something I'm missing here? I bypassed the problem by commenting out the entire maven-site-plugin plugin section of the distro profile in pom.xml. I have no idea what I just removed, but the build finishes now and the plugin seems to work.

@sabaka
Copy link
Contributor

sabaka commented Aug 21, 2014

Hi all!
I rebased my branch against checkstyle master. It has to work now.

Also thanks a lot for help with testing :)

@cbanack
Copy link

cbanack commented Aug 22, 2014

When I use my @sabaka clone (master branch), it still fails in the same spot, but with a slightly different error:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:38 min
[INFO] Finished at: 2014-08-21T17:57:00-06:00
[INFO] Final Memory: 62M/262M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.3:site (gen-site) on project checkstyle: Error during page generation: Error rendering Maven report:
[ERROR] Exit code: 1 - K:\GitHub\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\api\Check.java:29: error:
 invalid uri: "./{@docRoot}/../writingchecks.html"
[ERROR] * see <a href="./{@docRoot}/../writingchecks.html" target="_top">Writing
[ERROR] ^
[ERROR] K:\GitHub\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\api\TokenTypes.java:2283: error: bad HTM
L entity
[ERROR] * The <code>&</code> (bitwise AND) operator.
[ERROR] ^
[ERROR] K:\GitHub\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\api\TokenTypes.java:2283: warning: empty
 <code> tag
[ERROR] * The <code>&</code> (bitwise AND) operator.
[ERROR] ^
etc, etc, etc...

@ttjordan
Copy link

Is there any timeline for this issue?

@romani
Copy link
Member

romani commented Aug 23, 2014

@ttjordan , please see #10 (comment)

@sabaka
Copy link
Contributor

sabaka commented Aug 23, 2014

@cbanack why do you use master branch? It doesn't support Java 8 syntax. Please use gsoc branch.
As for failure, I'll check what's wrong asap.

@GreyTeardrop
Copy link

I have faced the same issue with mvn -Pdistro package build of gsoc branch as reported above (my build log). The issue should be reproducible by building Checkstyle with JDK 8 (I've used 1.8.0_20, Windows x64 version), as JDK 8 enables Javadoc linter by default.

The workaround is to add -Xdoclint:none to Java commandline arguments via top-level pom.xml:

<properties>
    …
    <additionalparam>-Xdoclint:none</additionalparam>
</properties>

@romani
Copy link
Member

romani commented Aug 26, 2014

@cbanack, please use java7 to build Checkstyle to avoid that problem easily.
I registered #261 to figure out what do with that violations - fix or ignore.

@cbanack
Copy link

cbanack commented Aug 26, 2014

@romani, thanks, I'll do that.

I recommend 'ignore', that way it behaves like Java 7. -Xdoclint SHOULD be set to none by default in Java 8. But it is not. This has introduced this exact same problem for nearly everyone else's projects, too. :(

@Pinny3
Copy link

Pinny3 commented Aug 28, 2014

+1

1 similar comment
@leocomelli
Copy link

👍

@romani
Copy link
Member

romani commented Sep 8, 2014

@sabaka , please rebase your changes from master, I fixed java8 javadoc problems in master.
That will ease test building for others.

@ikabiljo
Copy link

Two issues thus far with current code on gsoc branch:

Indentation properties don't seem to be applied properly to lambdas. For example, if I change indentation to 2, with:

    <module name="Indentation">
      <property name="caseIndent" value="0"/>
      <property name="basicOffset" value="2"/>
      <property name="braceAdjustment" value="0"/>
   </module>

It complains for the following code:

public class A {
  void function(Runnable x) {
    Runnable r = () -> {
      x.run();
      x.run();
    };
  }
}

wanting lambda to be 4 spaces indented:

block child at indentation level 6 not at correct indentation, 8
block child at indentation level 6 not at correct indentation, 8
block rcurly at indentation level 4 not at correct indentation, 6

Also static interface methods are not recognized properly:

public interface B {
  static int f() {
    int someName = 5;
    return someName;
  }
}

complains with:

Redundant 'static' modifier.
Name 'someName' must match pattern '^[A-Z][A-Z0-9]([A-Z0-9]+)_$'.

@romani
Copy link
Member

romani commented Sep 17, 2014

@ikabiljo, it is out of context of this branch fixes. Separate issue against IndentationCheck need to be created, please do (please provide examples in formatted way).

stevenschlansker pushed a commit to stevenschlansker/Singularity that referenced this issue Oct 2, 2014
@sabaka
Copy link
Contributor

sabaka commented Oct 3, 2014

Hi all!

My gsoc branch has been updated (Java 8 annotation here)

@T3rm1
Copy link

T3rm1 commented Oct 3, 2014

So, it's finished?

@sabaka
Copy link
Contributor

sabaka commented Oct 3, 2014

@T3rm1 almost. I'm cleaning build output, but it will not bring new changes into Java 8 support.

@ikabiljo
Copy link

ikabiljo commented Oct 4, 2014

Do you have an estimate when do you expect to be able to release it?
Is it in plan to fix most of outstanding issues with Java 8 support (like #281, #282, #284) before the release?

@romani
Copy link
Member

romani commented Oct 4, 2014

@ikabiljo,
#281 will not be touched for sure in next few weeks, we have huge changes in progress(maxvetrenko#26) for this Check, I do not what to interfere with it.
#282 , #284 - we could ask @sabaka to take a look, as it is not a complicated updates.

I plan to release java8 changes before 15 October.

@romani
Copy link
Member

romani commented Oct 10, 2014

finally merged to main repo, release 5.9 will be in few days.
Thanks a lot to all participants !!!!

@romani romani closed this as completed Oct 10, 2014
@benkiefer
Copy link

If anyone is facing this issue on gradle, you can bump the default version of checkstyle from 5.7 to 5.9 with the following config.

apply plugin: 'checkstyle'
checkstyle {
    toolVersion = "5.9"
}

You can see that config in the source here

@henrik242
Copy link

.. or rather 6.1.1 which is the latest version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests