Maxim Vetrenko. Project: Checkstyle configuration for Google's Java Style guide. GSoC 2014
Макс Ветренко
Project description: https://github.com/checkstyle/checkstyle/wiki/Checkstyle-GSoC-2014-Project-Ideas#project-name-checkstyle-configuration-for-googles-java-style-guide
Макс Ветренко
Monday report:
1) Detailed plan of practice is in progress.
2) Investigating Google's Java Style
Макс Ветренко
Ruslan Diachenko
On Tuesday, April 29, 2014 10:58:34 PM UTC+3, Макс Ветренко wrote:
Макс Ветренко
Roman Ivanov
Ruslan Diachenko
Roman Ivanov
Макс Ветренко
| Project Hosting on Google Code about some misunderstandings. |
Ruslan Diachenko
Макс Ветренко
Roman Ivanov
Roman Ivanov
Макс Ветренко
Roman Ivanov
Макс Ветренко
Ruslan Diachenko
Макс Ветренко
Roman Ivanov
Макс Ветренко
Roman Ivanov
Макс Ветренко
Roman Ivanov
Макс Ветренко
Ruslan Diachenko
Ruslan Diachenko
Макс Ветренко
Ruslan Diachenko
Макс Ветренко
Thursday report:
Questions from Roman:
1.” Why I do not see changes in plan ? that plan does not mean anything to me . Please udpate it.”
Answer:
Now my plan is updated. Please, check in: https://docs.google.com/document/d/1s1ymtee2leJYFG1xaeboQUWlGqGfz-54e7kI3hvGln4/edit#
2.
“http://google-styleguide.googlecode.com/svn/trunk/javaguide.html
is
"Last
changed: March 21, 2014"
what version you based on ?”
Answer:
Added link (https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#last-changed-in-google-java-style-march-21-2014) to the last changed file.
3. “"1.1 License or copyright information, if present"
What about http://checkstyle.sourceforge.net/config_header.html ? Checkstyle deman license header on all its sources - this is existing check.
If you not sure what Goolge expect from this rule - ask them :).”
Answer:
I found existing Check: https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#31-license-or-copyright-information-if-present
4. “that wikik is for users ,
Existing Check:LineLengthCheck.java.
so please remove references to javacode and put links to documentation HTML page.”
Answer:
Done.
5. “on each you attempt to create or update a existing Check - please consult with mentor.
http://checkstyle.sourceforge.net/config_imports.html#ImportOrder”
Answer:
I created some new issues and tonight I want to discuss it with Ruslan.
6. “"2.1 Exactly one top-level class declaration"
Why missed ?”
Answer:
7. “"2.1 Class member ordering"
http://checkstyle.sourceforge.net/config_coding.html#DeclarationOrder
Answer:
This case will be discussed tonight with Ruslan. After discussing I'll provide Skype Summary.
8. “"New Check: NoSplitOverloads"
discuss with a mentor”
Answer:
Created new issue to update existing Check: https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#3421-overloads-never-split
9. “please put at end of plan "Recheck all Checks at are used in GoogleStyle for open issues(github, sourceforge) and fix them"
Because Check could exist - but be extremely buggy :), and unusable in most cases, so known/registered problems have to be resolved.”
Answer:
Done. https://docs.google.com/document/d/1s1ymtee2leJYFG1xaeboQUWlGqGfz-54e7kI3hvGln4/edit# paragraph 2.9
10. “What does this phrase mean? http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s2.3.3-non-ascii-characters clearly show you UTF characters String in java code. How it could it non-compilable if it just String constant?”
Answer:
Founded existing Check: https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#233-non-ascii-characters
11. “All chapters from http://google-styleguide.googlecode.com/svn/trunk/javaguide.html have to exists at https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage with detailed information:
- what Check cover it,
- what could not be checked with wide reason description.
- link to issue to create new Check.
- if you do not know yet - there should be "?????" for that chapter.”
Answer:
Try to fix Wiki. Please, Check it. https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage
Questions from Ruslan:
1. “ Source file basics (http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s2-source-file-basics)
Why did you skip this section?”
Answer:
Added Source file basics into Wiki page. https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#2-source-file-basics
2. “ Need to ignore "^package" pattern
What did you mean by this? Why are you going to ignore pattern? Usually we ignore some values if they match some specific pattern :)”
Answer:
Sorry,
it's my mistake. We need to ignore “^package” and “^import”
values.
https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#32-package-statement
https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#332-no-line-wrapping
3. “I noticed some misunderstandings in Google's style guide, didn't you??
On the one hand "Wildcard imports, static or otherwise, are not used" (http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s3.3.1-wildcard-imports)
On the other hand "All static imports in a single group" (http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s3.3.3-import-ordering-and-spacing)”
Answer:
I have not answer from Google, so I want to offer next solution: static imports will be in the single group and another Check will warn not to use static imports. So, if developer needs static imports, he will ignore the Checkstyle warn.
4. “1.5 Ordering and spacing
What exactly are you going to update?”
Answer:
5. “2.2 Overloads: never split
Let's discuss”
Answer:
I want to update existing Check: https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#3421-overloads-never-split
6. “3.4 Block indentation: +2 spaces
Why is it skipped?”
Answer:
I found existing Check: https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#42-block-indentation-2-spaces
7. “3.5 One statement per line
Why is it skipped?”
Answer:
I found existing Check: https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage#43-one-statement-per-line
8. “Column limit: 80 or 100
In addition to package and import statements bear in mind these exceptions:
- Lines where obeying the column limit is not possible (for example, a long URL in Javadoc, or a long JSNI method reference).
- Command lines in a comment that may be cut-and-pasted into a shell.”
Answer:
I don't understand how to implement this rule, need to discuss.
9. “5.3 Static members: qualified using class
It seems that Checkstyle doesn't have such a rule. Why did you skip it? Please investigate if I'm mistaken.”
Answer:
We can't fully cover this rule. We can detect bad cases only if instance is created in the same class. For example:
class Foo {
public static void fooStaticMethod() {
//foo code
}
public void fooMethod() {
Foo foo= new Foo();
foo.fooStaticMethod(); //bad case
}
}
We can't detect bad using, if instance is adhere to another class. For example:
class Foo {
public static void fooStaticMethod() {
//foo code
}
public void fooMethod() {
Foo2 foo2= new Foo2();
foo2.fooStaticMethod2(); //bad case
}
}
Or Eclipse will warn, that “The static method fooStaticMethod() from the type Test should be accessed in a static way”
10. “2.1 License or copyright information, if present
Have a look at http://checkstyle.sourceforge.net/config_header.html#RegexpHeader. Can we play with it?”
Answer:
I think, yes. Regular expressions allow to detect license or copyright information.
11. “2.6.1 Exactly one top-level class declaration
Why did you skip it?”
Answer:
12. “4.7 Grouping parentheses: recommended
Why is it skipped?”
Answer:
needed to extra investigation.
13. “4.1 Rules common to all identifiers
Why is it empty?”
Answer:
Roman Ivanov
Макс Ветренко
Ruslan, I want to discuss with you next questions:
1. I don’t fully understand how to update DeclarationOrder and CustomDeclarationOrder Checks. https://github.com/maxvetrenko/checkstyle/wiki/Google%27s-Java-Style-Checkstyle-Coverage#342-class-member-ordering
2. I want to talk about static imports.
I don’t understand how to detect command lines or JSNI method reference. “ “Column limit: 80 or 100
In addition to package and import statements bear in mind these exceptions:
- Lines where obeying the column limit is not possible (for example, a long URL in Javadoc, or a long JSNI method reference).
- Command lines in a comment that may be cut-and-pasted into a shell.””
3. Need to discuss static members.
Roman Ivanov
Hi Max,
OrderCheck was developed by Baratali Izmailov, contact him for details if Ruslan in unavailable. Format of class is described there https://groups.google.com/d/msg/sevntu-checkstyle/_nOVtWDxPJo/sBb8s2D4lTwJ .
beware that this Check is sevntu project, if have to use it you need to initiate transferring it checkstyle project
For line length Check, you can check Guava code to see how they follow that convention for long comment lines. URL cannot be multilined, but may be Google demand event comments to follow that restriction 100 symbols. I recommend to ask Google about this and recheck Guava by LineLengthCheck ourself.
Please add in your plan for each Checks update to update html documentation on web site.
Roman Ivanov
Макс Ветренко
Roman Ivanov
Макс Ветренко
Tonight I'll write one more report.
Roman Ivanov
2) Update wiki to clearly state where update is required, format of description should be the same
3) Static import is not forbidden by google
4) Why some cells are empty in table for, symbols "--" (mean non relevant text) and arrow down (mean "parent chapter, see below") (http://en.wikipedia.org/wiki/%E2%86%93),
5) Generate html report (by usage of maven pluigin) over guava and provide link to it, fork guava repo to github to keep sources unchanged, report have to be stored at github (gh-pages). To all of that make a links from wiki. After each check implementation or configuration is done - report have to regenerated to show mentors how it works.
6) All questions "????" have to be resolved with me asap.
Roman Ivanov
Guide on how to create github pages https://guides.github.com/features/pages/
Макс Ветренко
Макс Ветренко
Макс Ветренко
Макс Ветренко
Ruslan Diachenko
On Wednesday, May 21, 2014 3:12:26 PM UTC+3, Макс Ветренко wrote:
Sorry.. Resolved https://github.com/maxvetrenko/checkstyle/blob/GoogleStyle/checkstyle_google_style.xml#L81
Макс Ветренко
Ruslan Diachenko
Roman Ivanov
Hi Max,
"test" prefix is only requirement of junit3 library that is not supported any more, junit4 library force to use annotation @Test on method.
Take a look at https://groups.google.com/forum/m/#!msg/sevntu-checkstyle/VBJlFhB0fuc/ey7y6RVomSgJ
.
Макс Ветренко
Макс Ветренко
Макс Ветренко
Roman Ivanov
Roman Ivanov
"svn checkout http://svn.apache.org/repos/asf/maven/plugins/tags/maven-checkstyle-plugin-2.12.1 maven-checkstyle-plugin"
do changes in pom.xml for <checkstyleVersion>5.7</checkstyleVersion> to <checkstyleVersion>5.8-SNAPSHOT</checkstyleVersion>
do "mvn install".
Макс Ветренко
Макс Ветренко
Макс Ветренко
Roman Ivanov
Ruslan Diachenko
Макс Ветренко
Roman Ivanov
Макс Ветренко
Roman Ivanov
Roman Ivanov
Макс Ветренко
Ruslan Diachenko
Ruslan Diachenko
Макс Ветренко
Roman, this is the answer for your proposal:
>> 3) we need to somehow markin that table :
https://github.com/maxvetrenko/checkstyle/wiki/Google's-Java-Style-Checkstyle-Coverage
I have done the update for Wiki page. Please, look at it. I think, that it doesn't make any sense to use some symbols or images for that. This is inconveniently to use symbols or images because reader have to know all definitions.
Also I have fix these mistakes:
>> Stable Guava Libraries. ===> Stable Guava source for v17.0.
>> The following issue already exists: https://github.com/sevntu-checkstyle/sevntu.checkstyle/issues/125 . So, in this issue we have to cover only the case with separators.
P.S. Ruslan, let's have a Skype call tonight, I have a question about JU method names.
Макс Ветренко
Ruslan Diachenko
Roman Ivanov
Макс Ветренко
Ruslan Diachenko
Макс Ветренко
1) I think, that commet checking doesn't give us any result. Commet can't give us any assurance.
2) It doesn't necessary to cover import and package declaration, because we can't declare package after import. It will be a syntax error. Import ordering is implemented in 3.3.3 Ordering and spacing. So we have to cover license declaration and Exactly one blank line separates each section that is present. Line separates is not available in Checkstyle, so we have to implement new Check: ExactlyOneBlankLineSeparate. In this note I have only one question. It's about License. What Check we have to use? Header or RedexpHeader?
3) Google says, that wildcard imports, static or otherwise are not used. And existing Check covers all requirements.
4) Done.
5) Done.
6) ???????? Need to discuss
7) Done.
8) Done.
9) MethodNameAwareOfJunitExtendedCheck - new Check to cover http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s5.2.3-method-names . Investigate how junit 3 detect test methods.
10) Done.
Update Wiki. Added markers.
Skype summary with Roman:
We discussed Unit method and we concluded, that it's necessary to create new Check MethodNameAwareOfJunitExtendedCheck to cover rule. Check will detect JUnit4 method by @test, and JUnit3 method by class name, extended class, imports (junit.framework) and modifiers (public void, maybe final) .
Макс Ветренко
Roman Ivanov
Roman Ivanov
Roman Ivanov
Макс Ветренко
Ruslan Diachenko
Ruslan Diachenko
Макс Ветренко
>>Yes, it doesn't help us in this case
Макс Ветренко
Roman Ivanov
Макс Ветренко
Compare JUnit3 and JUnit4. My investigations:
1. JUnit3:
a. JUnit3 class have to import junit.framework.TestCase
b. JUnit test must be public void and test name have to start with "test".
c. Test class must extend TestCase.
2. JUnit4:
a. Test method must have annotation @Test.
b. Test Class can include next annotations: @Test, @Before, @After, @BeforeClass, @AfterClass, @Ignore.
c. JUnit4 Test Class have imports as org.junit.*
All this investigation takes an hour, because I investigated different cases and I think, that it's enough to detect JUnit3 and JUnit4 test methods and classes. We have to provide new Check to cover JUnit method names. If user wants to override at-clause annotation, future Check will have ability to work with custom annotations. For JUnit3: if user extends some class that extends TestCase, Check will have ability to work with such classes.
We have familiar existing issue: https://github.com/sevntu-checkstyle/sevntu.checkstyle/issues/80
So, today I''ll investigate this Check and try to make it work.
Макс Ветренко
Roman Ivanov
Roman Ivanov
Макс Ветренко
Макс Ветренко
Roman Ivanov
Макс Ветренко
Roman Ivanov
do it.
Roman Ivanov
Roman Ivanov
you need to split implementation in two phases:
2) Update plan:
4.6.1 - incorrect mapping to check. - RegExp can not do that.
4.7 and 4.6.3 are not a rules and just optional recomendations - marked differently in table. The same marking should be used.
4.8.3.1 - incorrect Check usage - There is no reference to "," in Google's style.
4.8.5 - incorrect Check usage - I do not understand how you are going to cover that guides by that Check.
4.8.6.1- incorrect Check usage - it is impossible to do now by Checkstyle.
Roman Ivanov
Roman Ivanov
<reporting><plugins><plugin><groupId>org.apache.maven.plugins</groupId><artifactId>maven-checkstyle-plugin</artifactId><version>2.12.1</version><configuration><configLocation>checkstyle_google_style.xml</configLocation><enableFilesSummary>false<enableFilesSummary></configuration></plugin></plugins></reporting>Sources:thanks,Roman Ivanov
Макс Ветренко
Roman Ivanov
Roman Ivanov
Roman Ivanov
private Pattern mValidTestClassNameRegex = Pattern.compile(".+Test|Test.+|.+IT|.+TestCase");Names of methods could contain following symbols:
A-Z
(\u0041-\u005a), and a-z
(\u0061-\u007a), and, for historical reasons, the
ASCII underscore (_, or \u005f)
and dollar sign ($,
or \u0024). The $ character
should be used only in mechanically generated source code or, rarely,
to access pre-existing names on legacy systems.