chore: add check-style plugin #1810

seagle-yuan · 2022-04-07T03:27:11Z

Add checkstyle plugin
implement #735

TODO:

forced to check with error level after fix style issues

CLAassistant · 2022-04-07T03:27:20Z

All committers have signed the CLA.

github-actions · 2022-04-07T03:27:24Z

CLA Assistant Lite bot Good! All Contributors have signed the CLA.

seagle-yuan · 2022-04-07T03:51:12Z

I had signed CLA

javeme · 2022-04-07T04:01:39Z

style/checkstyle.xml

+    </module>
+
+    <module name="LineLength">
+        <property name="max" value="200"/>


Need to merge our own style rule, we can refer to https://github.com/hugegraph/hugegraph-common/pull/67/files#diff-b36112ff71ee5b411c9149781cee470c6413dbf498272fa70eb00b8b7555207fR22

javeme · 2022-04-07T04:02:19Z

style/intellij-java-code-style.xml

+  <codeStyleSettings language="JAVA">
+    <option name="BINARY_OPERATION_SIGN_ON_NEXT_LINE" value="true" />
+  </codeStyleSettings>
+</code_scheme>


expect an empty line at end of file

seagle-yuan · 2022-04-07T05:01:24Z

I have read the CLA Document and I hereby sign the CLA

seagle-yuan · 2022-04-11T03:06:16Z

hugegraph checkstyle的对比

一、插件版本

hugegraph-common 3.1.0
dolphinscheduler 3.1.2
建议使用3.1.2。两个版本配置上稍微有点区别，hugegraph-common中的checkstyle.xml不能直接在高版本中使用。需要调整部分model的依赖级别

二、注释方面

hugegraph-common 采用了大量中文注释，虽然便于国内社区人员理解，但是会影响hugegraph在全球传播。
dolphinscheduler 注释误解较多，应该是某位同学大量复制并未仔细检查造成的。
建议不使用注释，或者注释全部参考官方注释，保证无歧义 https://checkstyle.sourceforge.io/

三、具体参数对比

“无” 表示无此项配置

“默认” 表示该项配置使用默认值

对比情况“一致”表示双方配置一样，或者配置使用效果一样。“补充”表示dolphinscheduler中由此配置，但是hugegraph中无此项配置。"有冲突"表示双方均有此项配置，且不一致。

子模块名称	hugegraph版本	dolphinscher版本	对比情况	说明
FileTabCharacter	eachLine:true	eachLine:true	一致
LineLength	最大长度：80 ignorePattern：^package.\| ^import.\| a href\|href\| http://\| https://\|ftp://	最大长度：200 ignorePattern：^ \ *[^ ]+$	有冲突	限制每行字符长度
RegexpSingleline	无	format:System\.out\.println	补充	根据正则表达式检查单行是否有不良操作
PackageName	format: ^[a-z]+(\.[a-z][a-z0-9])$	format:^[a-z]+(\.[a-z][a-z0-9])$	一致	检查包名称是否遵守命名规约
ImportOrder	无	staticGroups: org.apache.dolphinscheduler, org.apache,java,javax,org,com separatedStaticGroups:true groups:org.apache.dolphinscheduler, org.apache,java,javax,org,com ordered:true separated:true option:top sortStaticImportsAlphabetically:true	补充	定义导入包的顺序
AvoidStarImport	默认	默认	一致	检查没有import语句使用*号
RedundantImport	默认	默认	一致	检查是否存在多余的import语句，比如重复的，java自带的包，相同包下的其他类
UnusedImports	默认	默认	一致	检查是否存在没有使用的import语句
EmptyLineSeparator	无	allowMultipleEmptyLines:false allowMultipleEmptyLinesInsideClassMembers: false tokens:PACKAGE_DEF, IMPORT, STATIC_IMPORT, CLASS_DEF,INTERFACE_DEF, ENUM_DEF, STATIC_INIT, INSTANCE_INIT, METHOD_DEF,CTOR_DEF	补充	检查空行是否符合要求。案例https://checkstyle.sourceforge.io/config_whitespace.html#EmptyLineSeparator
ModifierOrder	默认	默认	一致	检查修饰符是否符合Java建议，顺序是： public、protected、private、abstract、default、static、final、transient、volatile、synchronized、native、strictfp
TypeName	无	默认	补充	检查类名称是否符合规范
ClassTypeParameterName	无	format: (^[A-Z][0-9]?)$\|([A-Z][a-zA-Z0-9]*[T]$)	补充	检查泛型类类型参数是否符合规范
ConstantName	format: ^[A-Z][A-Z0-9](_[A-Z0-9]+)$	无	ds缺失	检查常量（用static final修饰的字段）的名称是否遵守命名规约
LocalVariableName	format: ^[a-z][a-zA-Z0-9_]*$	tokens:VARIABLE_DEF format:^[a-z]([a-zA-Z0-9]*)?$	有冲突	检查局部变量的名称是否遵守命名规约。 dolphinscheduler不允许有下划线 hugegraph下可以有下划线
MemberName	format: ^[a-z][a-zA-Z0-9]*$	format:^[a-z][a-zA-Z0-9]*$	一致	检查成员变量（非静态字段）的名称是否遵守命名规约
ParameterName	format: ^[a-z][a-zA-Z0-9]*$	format:^[a-z]([a-zA-Z0-9]*)?$	一致	检查参数名称是否遵守命名规约
CatchParameterName	无	format:^[a-z]([a-zA-Z0-9]*)?$	补充	检查catch参数名是否符合要求
ArrayTypeStyle	javaStyle:true	默认	一致	检查数组是否属于java风格，方括号放在类型后面，而不是变量后面，比如：int[] nums（合法），int nums[]（不合法）
MethodTypeParameterName	无	format: (^[A-Z][0-9]?)$\|([A-Z][a-zA-Z0-9]*[T]$)	补充	检查方法类型是否遵守命名规约
MethodName	format: ^[a-z][a-zA-Z0-9]*$	format:^[a-z][a-z0-9][a-zA-Z0-9_]*$	有冲突	检查方法名称是否遵守命名规约 dolphinscheduler不允许有下划线 hugegraph下可以有下划线
InterfaceTypeParameterName	无	format: (^[A-Z][0-9]?)$\|([A-Z][a-zA-Z0-9]*[T]$)	补充
EqualsHashCode	默认	无	ds缺失	覆盖equals()方法的类必须也覆盖了hashCode()方法
MissingSwitchDefault	默认	无	ds缺失	switch语句必须含有default子句
DefaultComesLast	默认	无	ds缺失	switch语句的default必须放在所有的case分支之后
SuperClone	默认	无	ds缺失	覆盖clone()方法时调用了super.clone()方法
NoFinalizer	无		补充	验证类中是否定义了finalize()方法。
GenericWhitespace	无	默认	补充	检查范型标记<>的周围的空格是否遵守标准规约 https://checkstyle.sourceforge.io/config_whitespace.html#GenericWhitespace
Indentation	无	basicOffset:4 braceAdjustment:0 caseIndent:4 throwsIndent:2 lineWrappingIndentation:4 arrayInitIndent:4	补充	检查Java代码的缩进是否正确。
UpperEll	默认	默认	一致	long类型的字面量如果要以"L"结尾，必须是大写的"L"，而非小写的"l"
IllegalImport	无	regexp:4 illegalPkgs: ^com\.google\.api\.client\.repackaged, ^avro\.shaded, ^org\.apache\.hadoop\.hbase\.shaded, ^org\.apache\.hadoop\.shaded, ^javax\.ws\.rs\.ext, ^cc\.concurrent\.mango\.util\.concurrent, ^org\.apache\.curator-test\.shaded, ^com\.sun\.istack, ^org\.jetbrains\.annotations, ^jline\.internal, ^com\.cronutils\.utils, ^javax\.ws\.rs\.ext, ^org\.jboss\.netty\.util\.internal, ^com\.sun\.javafx, ^io\.reactivex\.annotations, ^org\.codehaus\.jackson	补充	检查是否有非法导入的包与类
NoWhitespaceBefore	无	tokens: COMMA, SEMI, POST_INC, POST_DEC, DOT, ELLIPSIS, METHOD_REF allowLineBreaks:true	补充	检查指定标记之前没有空格。
ParenPad	默认	默认	一致	左圆括号之后和右圆括号之前是否需要有一个空格，不需要
OperatorWrap	option:eol	option:NL tokens: BAND, BOR, BSR, BXOR, DIV, EQUAL, GE, GT, LAND, LE,LITERAL_INSTANCEOF, LOR, LT, MINUS, MOD, NOT_EQUAL, PLUS, QUESTION, SL, SR, STAR, METHOD_REF	有冲突	代码换行时，运算符位置，比如：+、&&、?、: 等 dolphinscheduler要求在下一行行首 hugegraph 要求在本行末尾
EmptyStatement	默认	severity:error	有冲突	代码中不允许有空语句，也就是单独的;符号 dolphinscheduler将该错误级别置为error
AnnotationLocation	无	allowSamelineMultipleAnnotations:false allowSamelineSingleParameterlessAnnotation: false allowSamelineParameterizedAnnotation:true tokens:METHOD_DEF, CTOR_DEF	补充	检查注解位置是否符合规范
EmptyCatchBlock	无	exceptionVariableName:expected	补充	检查空的catch块。
CommentsIndentation	无	默认	补充	检查缩进与代码是否一致
NoLineWrap	无	severity:error	补充	检查（默认import & package）是否换行 dolphinscheduler将该错误级别置为error
NeedBraces	无	severity:error	补充	检查是否需要大括号，在if lese等场景使用 dolphinscheduler将该错误级别置为error
LeftCurly	option:eol ignoreEnums:false	severity:error	有冲突	检查{和左边的代码是否在同行 dolphinscheduler将该错误级别置为error
WhitespaceAround	默认	allowEmptyConstructors:true allowEmptyMethods:true allowEmptyTypes:true allowEmptyLoops:true	有冲突	检查指定标记的周围是否有空格，比如：if、for、while、synchoronized 等
AvoidNestedBlocks	无	allowInSwitchCase:true	补充	检查是否有嵌套块 (允许switch语句中使用)
IllegalTokenText	无	tokens:STRING_LITERAL, CHAR_LITERAL format: \\u00(09\|0(a\|A)\|0(c\|C)\|0(d\|D)\|22\|27\|5(C\|c))\| \\(0(10\|11\|12\|14\|15\|42\|47)\|134)	补充	检查文本内容是否非法字符
AvoidEscapedUnicodeCharacters	无	allowEscapesForControlCharacters:true allowByTailComment:true allowNonPrintableEscapes:true	补充	限制使用Unicode escapes
EmptyBlock	无	option:TEXT tokens:LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH	补充	检查空块
OneStatementPerLine	无	allowNonPrintableEscapes:true	补充	确保每行只有一条语句
MultipleVariableDeclarations	无	默认	补充	检查代码段和行中是否有多次变量声明
MissingSwitchDefault	无	默认	补充	检测switch语句里是否有默认部分
FallThrough	无	默认	补充	检查switch是否缺少break、continue、return、throw
JavadocStyle	无	endOfSentenceFormat:	补充	自定义检查以验证javadoc
JavadocType	无	scope:protected allowMissingParamTags:true	补充	检查javadoc的类型

imbajin · 2022-04-12T14:51:25Z

style/intellij-java-code-style.xml

+<code_scheme name="hugegraph-style" version="173">
+  <option name="LINE_SEPARATOR" value="&#xA;" />
+  <option name="RIGHT_MARGIN" value="80" />


seems we already have one code-style file (in dir root), u could update that directly

Thank you for your reminder,I have noticed this problems.
first i will delete "style/intellij-java-code-style.xml"
and then can you review "code/checkstyle.xml"

OK, if u update in the old file, we could see the diff easily

javeme · 2022-04-14T03:51:55Z

@seagle-yuan Thank you very much, your sorting is very clear.

There are 2 conflicts I think can keep the existing style:

LineLength: Prefer to keep line length 80 chars, because too long code can lead to poor readability. Maybe it can be relaxed to 100 characters later if necessary.
OperatorWrap: The operator location is placed at the end of the line when wrapping line, we consider that when we read the code, it means "it's not over yet, please continue to the next line".

IMO the other 5 conflicts can follow the style of dolphinscheduler.

javeme · 2022-04-14T13:09:15Z

style/checkstyle.xml

+
+
+


one blank line is ok

wait a minute,let me resubmit it.

I had deleted unnecessary balnk lines.

codecov · 2022-04-14T13:16:57Z

Codecov Report

Merging #1810 (c5d0075) into master (0a3599d) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1810   +/-   ##
=========================================
  Coverage     70.90%   70.90%           
  Complexity      980      980           
=========================================
  Files           446      446           
  Lines         37902    37902           
  Branches       5398     5398           
=========================================
  Hits          26876    26876           
  Misses         8296     8296           
  Partials       2730     2730

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 0a3599d...c5d0075. Read the comment docs.

javeme · 2022-04-15T05:42:29Z

recheck

javeme · 2022-04-15T06:22:26Z

style/checkstyle.xml

+        <module name="MissingSwitchDefault"/>
+        <module name="FallThrough"/>
+        <module name="OuterTypeFilename">
+<!--            <property name="severity" value="error"/>-->


should enable after style issues are all fixed?

javeme · 2022-04-15T06:27:02Z

style/checkstyle.xml

+    </module>
+    <module name="LineLength">
+        <property name="max" value="100"/>
+        <property name="ignorePattern" value="^ *\* *[^ ]+$"/>


I used the different property to test and got the same result。so i think we can change this property at the next version.

javeme · 2022-04-15T06:28:57Z

style/checkstyle.xml

+        "https://checkstyle.org/dtds/configuration_1_3.dtd">
+<module name="Checker">
+    <property name="charset" value="UTF-8"/>
+    <property name="severity" value="info"/>


should set to error after style issues are all fixed?

imbajin

Thanks for your contribution, LGTM

imbajin · 2022-04-15T13:05:52Z

style/checkstyle.xml

+            <property name="allowSamelineSingleParameterlessAnnotation"
+                      value="false"/>


seems we could merge 154 to 153

imbajin

Merge first, enhance it in another pr

add checkstyle plugin

1f04187

trigger CLA check

d06d7e6

javeme reviewed Apr 7, 2022

View reviewed changes

github-actions bot added a commit that referenced this pull request Apr 7, 2022

@seagle-yuan has signed the CLA from Pull Request #1810

6c427e1

merge checkstyle and intellij-java-code-style

1d603b3

imbajin reviewed Apr 12, 2022

View reviewed changes

seagle-yuan and others added 2 commits April 14, 2022 20:54

update checkstyle.xml

f096b98

Merge branch 'hugegraph:master' into checkstyle-branch

96c3ab5

javeme reviewed Apr 14, 2022

View reviewed changes

delete unnecessary blank lines

8d57a41

seagle-yuan requested review from javeme and imbajin April 15, 2022 01:15

commented checkstyle ImportOrder module

c5d0075

github-actions bot added a commit that referenced this pull request Apr 15, 2022

@seagle-yuan has signed the CLA from Pull Request #1810

6210c8f

javeme approved these changes Apr 15, 2022

View reviewed changes

imbajin reviewed Apr 15, 2022

View reviewed changes

imbajin changed the title ~~add checkstyle plugin~~ chore: add check-style plugin Apr 16, 2022

imbajin approved these changes Apr 16, 2022

View reviewed changes

imbajin merged commit c904dcb into apache:master Apr 16, 2022

seagle-yuan deleted the checkstyle-branch branch April 17, 2022 03:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add check-style plugin #1810

chore: add check-style plugin #1810

seagle-yuan commented Apr 7, 2022 •

edited

Loading

CLAassistant commented Apr 7, 2022 •

edited

Loading

github-actions bot commented Apr 7, 2022 •

edited

Loading

seagle-yuan commented Apr 7, 2022

javeme Apr 7, 2022

javeme Apr 7, 2022

seagle-yuan commented Apr 7, 2022

seagle-yuan commented Apr 11, 2022 •

edited

Loading

imbajin Apr 12, 2022

seagle-yuan Apr 13, 2022

imbajin Apr 13, 2022

javeme commented Apr 14, 2022

javeme Apr 14, 2022

seagle-yuan Apr 14, 2022

seagle-yuan Apr 14, 2022

codecov bot commented Apr 14, 2022 •

edited

Loading

javeme commented Apr 15, 2022

javeme Apr 15, 2022

javeme Apr 15, 2022

seagle-yuan Apr 15, 2022

javeme Apr 15, 2022

imbajin left a comment •

edited

Loading

imbajin Apr 15, 2022

imbajin left a comment

		<property name="allowSamelineSingleParameterlessAnnotation"
		value="false"/>

chore: add check-style plugin #1810

chore: add check-style plugin #1810

Conversation

seagle-yuan commented Apr 7, 2022 • edited Loading

CLAassistant commented Apr 7, 2022 • edited Loading

github-actions bot commented Apr 7, 2022 • edited Loading

seagle-yuan commented Apr 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seagle-yuan commented Apr 7, 2022

seagle-yuan commented Apr 11, 2022 • edited Loading

hugegraph checkstyle的对比

一、插件版本

二、注释方面

三、具体参数对比

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javeme commented Apr 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 14, 2022 • edited Loading

Codecov Report

javeme commented Apr 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imbajin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imbajin left a comment

Choose a reason for hiding this comment

seagle-yuan commented Apr 7, 2022 •

edited

Loading

CLAassistant commented Apr 7, 2022 •

edited

Loading

github-actions bot commented Apr 7, 2022 •

edited

Loading

seagle-yuan commented Apr 11, 2022 •

edited

Loading

codecov bot commented Apr 14, 2022 •

edited

Loading

imbajin left a comment •

edited

Loading