Re: Add more rules to checkstyle.xml in fe

2022-04-28 Thread morrysnow
Hi, devs,

I split adding java check style into 5 steps. I’m willing to get feedback about 
it.

CheckStyle and Spotless will modify about 10k lines.

Step 1:

1. Add Spotless plugin and do some simple format(1198 files changed, 3205 
insertions(+), 4350 deletions(-))
a. end with new line
b. trim trailing whitespace
c. replace tab indent with 4 spaces indent
d. license header check
e. adjust the order of import to "static, org.apache.doris, third 
party, java"
f. remove unused imports
g. make sure all files are encoding by utf-8
h. make sure all files line ending by LF

2. Add checkstyle rules but set default severity to warning to avoid compile 
failed.

3. set some rules' severity to error in Checkstyle since no lines in doris 
break these rules(0 files, 0 lines)
a. Merge conflicts unresolved
b. Avoid using corresponding octal or Unicode escape
c. Avoid Escaped Unicode Characters
d. No Line Wrap
e. Package Name
f. Type Name
g. Annotation Location
h. Interface Type Parameter 
i. CatchParameterName
j. Pattern Variable Name
k. Record Component Name
l. Record Type Parameter Name
m. Method Type Parameter Name

4. Set below rules severity to error in Checkstyle since these had done by 
splotless(12 files, 177 lines)
a. Redundant Import
b. Custom Import Order
c. Unused Imports
d. Avoid Star Import
e. tab character in file
f. Newline At End Of File
g. Trailing whitespace found



Step 2: (241 files, 962 lines (with Step 1))

1. Set below rules' severity to error in Checkstyle, all rules are import to 
make sure the code is correct
a. Need Braces
b. Equals Hash Code
c. Missing Switch Default
d. Fall Through
e. No Finalizer


2. Set below rules' severity to error in Checkstyle, all rules are about name
a. Member Name
b. Constant Name
c. Parameter Name
d. LambdaParameterName
e. Local Variable Name
f. Class Type Parameter Name
g. Static Variable Name
h. Method Name
j. Abbreviation As Word In Name


Step 3: (605 files, 6008 lines (with Step 1 and Step 2))

1. Set below rules' severity to error in Checkstyle, all rules are about 
whitespace
a. Empty Block
b. Left Curly
c. Right Curly
d. White space Around
e. Generic Whitespace
f. Indentation
g. No Whitespace Before Case Default Colon
h. Method Param Pad
i. Whitespace
j. Paren Pad
k. Extra newline
l. Extra newline at end of block
m. Empty Statement
n. Empty Catch Block
o. Comments Indentation

2. Set below rules' severity to error in Checkstyle, all rules are about the 
position of the newline
a. Line Length 120
b. Operator Wrap
c. One Statement Per Line
d. Multiple Variable Declarations


Step 4: (674 files, 6730 lines (with Step 1, Step 2 and Step 3))

1. Set below rules' severity to error in Checkstyle
a. Array Type Style
b. Upper Ell
c. Modifier Order
d. Empty Line Separator
e. Separator Wrap
f. Overload Methods Declaration Order
g. Variable Declaration Usage Distance
i. One Top Level Class


Step 5:

1. Add java doc check to CheckStyle and set severity to warning.

2. Fix java doc warning gradually


> 2022年4月28日 00:39,morrysnow  写道:
> 
> Hi, devs
> 
> I summarized all the rules I wanted to add. I examined the number of lines of 
> code affected by all the rules and categorized them by importance.
> 
> The link is here: 
> https://github.com/apache/incubator-doris/issues/8985#issuecomment-214162 
> 
> 
> I would like to be able to submit them in multiple patches. The principle of 
> patch splitting is as follows. Those with low impact and high importance are 
> submitted first and sets the severity to Error. The Javadoc commits last and 
> sets the severity to Warning.
> 
> 
>> 2022年4月18日 16:09,vin jake  写道:
>> 
>> Hi, there is a new PR about format doris.
>> https://github.com/apache/incubator-doris/pull/9072
>> 
>> I think it's time to ensure what checkstyle rules  we should add for fe.
>> 
>> All people who care about format rule can put up your thoughts.
>> 
>> We can add it in the issue:
>> https://github.com/apache/incubator-doris/issues/8985
>> 
>> 
>> On Thu, Apr 14, 2022 at 6:52 PM vin jake  wrote:
>> 
>>> great, I will trim it in my PR.
>>> 
>>> checkstyle.xml need more discussion
>>> 
>>> On Thu, Apr 14, 2022 at 5:48 PM morrysnow  wrote:
>>> 
 I agree with u.
 
 For the first point, I want to list rules first, and then change
 checkstyle.xml.
 
 For the second point, original change information in git is no

Re:Re: Add more rules to checkstyle.xml in fe

2022-04-28 Thread 陈明雨
That would be great!
Some suggestion about the rules:


> d. license header check
For now, we use skywalking-eyes to check license header, is this rule still 
needed?


> adjust the order of import to "static, org.apache.doris, third party, java"
Better not using static import




--

此致!Best Regards
陈明雨 Mingyu Chen

Email:
chenmin...@apache.org





在 2022-04-28 17:14:34,"morrysnow"  写道:
>Hi, devs,
>
>I split adding java check style into 5 steps. I’m willing to get feedback 
>about it.
>
>CheckStyle and Spotless will modify about 10k lines.
>
>Step 1:
>
>1. Add Spotless plugin and do some simple format(1198 files changed, 3205 
>insertions(+), 4350 deletions(-))
>   a. end with new line
>   b. trim trailing whitespace
>   c. replace tab indent with 4 spaces indent
>   d. license header check
>   e. adjust the order of import to "static, org.apache.doris, third 
> party, java"
>   f. remove unused imports
>   g. make sure all files are encoding by utf-8
>   h. make sure all files line ending by LF
>
>2. Add checkstyle rules but set default severity to warning to avoid compile 
>failed.
>
>3. set some rules' severity to error in Checkstyle since no lines in doris 
>break these rules(0 files, 0 lines)
>   a. Merge conflicts unresolved
>   b. Avoid using corresponding octal or Unicode escape
>   c. Avoid Escaped Unicode Characters
>   d. No Line Wrap
>   e. Package Name
>   f. Type Name
>   g. Annotation Location
>   h. Interface Type Parameter 
>   i. CatchParameterName
>   j. Pattern Variable Name
>   k. Record Component Name
>   l. Record Type Parameter Name
>   m. Method Type Parameter Name
>
>4. Set below rules severity to error in Checkstyle since these had done by 
>splotless(12 files, 177 lines)
>   a. Redundant Import
>   b. Custom Import Order
>   c. Unused Imports
>   d. Avoid Star Import
>   e. tab character in file
>   f. Newline At End Of File
>   g. Trailing whitespace found
>
>
>
>Step 2: (241 files, 962 lines (with Step 1))
>
>1. Set below rules' severity to error in Checkstyle, all rules are import to 
>make sure the code is correct
>   a. Need Braces
>   b. Equals Hash Code
>   c. Missing Switch Default
>   d. Fall Through
>   e. No Finalizer
>
>
>2. Set below rules' severity to error in Checkstyle, all rules are about name
>   a. Member Name
>   b. Constant Name
>   c. Parameter Name
>   d. LambdaParameterName
>   e. Local Variable Name
>   f. Class Type Parameter Name
>   g. Static Variable Name
>   h. Method Name
>   j. Abbreviation As Word In Name
>
>
>Step 3: (605 files, 6008 lines (with Step 1 and Step 2))
>
>1. Set below rules' severity to error in Checkstyle, all rules are about 
>whitespace
>   a. Empty Block
>   b. Left Curly
>   c. Right Curly
>   d. White space Around
>   e. Generic Whitespace
>   f. Indentation
>   g. No Whitespace Before Case Default Colon
>   h. Method Param Pad
>   i. Whitespace
>   j. Paren Pad
>   k. Extra newline
>   l. Extra newline at end of block
>   m. Empty Statement
>   n. Empty Catch Block
>   o. Comments Indentation
>
>2. Set below rules' severity to error in Checkstyle, all rules are about the 
>position of the newline
>   a. Line Length 120
>   b. Operator Wrap
>   c. One Statement Per Line
>   d. Multiple Variable Declarations
>
>
>Step 4: (674 files, 6730 lines (with Step 1, Step 2 and Step 3))
>
>1. Set below rules' severity to error in Checkstyle
>   a. Array Type Style
>   b. Upper Ell
>   c. Modifier Order
>   d. Empty Line Separator
>   e. Separator Wrap
>   f. Overload Methods Declaration Order
>   g. Variable Declaration Usage Distance
>   i. One Top Level Class
>
>
>Step 5:
>
>1. Add java doc check to CheckStyle and set severity to warning.
>
>2. Fix java doc warning gradually
>
>
>> 2022年4月28日 00:39,morrysnow  写道:
>> 
>> Hi, devs
>> 
>> I summarized all the rules I wanted to add. I examined the number of lines 
>> of code affected by all the rules and categorized them by importance.
>> 
>> The link is here: 
>> https://github.com/apache/incubator-doris/issues/8985#issuecomment-214162
>>  
>> 
>> 
>> I would like to be able to submit them in multiple patches. The principle of 
>> patch splitting is as follows. Those with low impact and high importance are 
>> submitted first and sets the severity to Error. The Javadoc commits last and 
>> sets the severity to Warning.
>> 
>> 
>>> 2022年4月18日 16:09,vin jake  写道:
>>> 
>>> Hi, there is a new PR about format doris.
>>> https://github.com/apache/incubator-doris/pull/9072
>>> 
>>> I think it's time to ensure what checkstyle rules  we should add for fe.
>>> 
>>> All people who care about format rule can put up your thoughts.
>>> 
>>> We can 

[NOTICE] BE code format

2022-04-28 Thread 陈明雨
Hi all,
I will merge this PR[1] today to format all BE code using clang.
And aslo enable the code format check as required check.


[1] https://github.com/apache/incubator-doris/pull/9305




--

此致!Best Regards
陈明雨 Mingyu Chen

Email:
chenmin...@apache.org

Re: Add more rules to checkstyle.xml in fe

2022-04-28 Thread morrysnow
Hi, mingyu

1. Check license is needed for local check.
2. Add a rule to forbidden static import is ok for me

> 2022年4月28日 21:34,陈明雨  写道:
> 
> That would be great!
> Some suggestion about the rules:
> 
> 
>> d. license header check
> For now, we use skywalking-eyes to check license header, is this rule still 
> needed?
> 
> 
>> adjust the order of import to "static, org.apache.doris, third party, java"
> Better not using static import
> 
> 
> 
> 
> --
> 
> 此致!Best Regards
> 陈明雨 Mingyu Chen
> 
> Email:
> chenmin...@apache.org
> 
> 
> 
> 
> 
> 在 2022-04-28 17:14:34,"morrysnow"  写道:
>> Hi, devs,
>> 
>> I split adding java check style into 5 steps. I’m willing to get feedback 
>> about it.
>> 
>> CheckStyle and Spotless will modify about 10k lines.
>> 
>> Step 1:
>> 
>> 1. Add Spotless plugin and do some simple format(1198 files changed, 3205 
>> insertions(+), 4350 deletions(-))
>>  a. end with new line
>>  b. trim trailing whitespace
>>  c. replace tab indent with 4 spaces indent
>>  d. license header check
>>  e. adjust the order of import to "static, org.apache.doris, third 
>> party, java"
>>  f. remove unused imports
>>  g. make sure all files are encoding by utf-8
>>  h. make sure all files line ending by LF
>> 
>> 2. Add checkstyle rules but set default severity to warning to avoid compile 
>> failed.
>> 
>> 3. set some rules' severity to error in Checkstyle since no lines in doris 
>> break these rules(0 files, 0 lines)
>>  a. Merge conflicts unresolved
>>  b. Avoid using corresponding octal or Unicode escape
>>  c. Avoid Escaped Unicode Characters
>>  d. No Line Wrap
>>  e. Package Name
>>  f. Type Name
>>  g. Annotation Location
>>  h. Interface Type Parameter 
>>  i. CatchParameterName
>>  j. Pattern Variable Name
>>  k. Record Component Name
>>  l. Record Type Parameter Name
>>  m. Method Type Parameter Name
>> 
>> 4. Set below rules severity to error in Checkstyle since these had done by 
>> splotless(12 files, 177 lines)
>>  a. Redundant Import
>>  b. Custom Import Order
>>  c. Unused Imports
>>  d. Avoid Star Import
>>  e. tab character in file
>>  f. Newline At End Of File
>>  g. Trailing whitespace found
>> 
>> 
>> 
>> Step 2: (241 files, 962 lines (with Step 1))
>> 
>> 1. Set below rules' severity to error in Checkstyle, all rules are import to 
>> make sure the code is correct
>>  a. Need Braces
>>  b. Equals Hash Code
>>  c. Missing Switch Default
>>  d. Fall Through
>>  e. No Finalizer
>> 
>> 
>> 2. Set below rules' severity to error in Checkstyle, all rules are about name
>>  a. Member Name
>>  b. Constant Name
>>  c. Parameter Name
>>  d. LambdaParameterName
>>  e. Local Variable Name
>>  f. Class Type Parameter Name
>>  g. Static Variable Name
>>  h. Method Name
>>  j. Abbreviation As Word In Name
>> 
>> 
>> Step 3: (605 files, 6008 lines (with Step 1 and Step 2))
>> 
>> 1. Set below rules' severity to error in Checkstyle, all rules are about 
>> whitespace
>>  a. Empty Block
>>  b. Left Curly
>>  c. Right Curly
>>  d. White space Around
>>  e. Generic Whitespace
>>  f. Indentation
>>  g. No Whitespace Before Case Default Colon
>>  h. Method Param Pad
>>  i. Whitespace
>>  j. Paren Pad
>>  k. Extra newline
>>  l. Extra newline at end of block
>>  m. Empty Statement
>>  n. Empty Catch Block
>>  o. Comments Indentation
>> 
>> 2. Set below rules' severity to error in Checkstyle, all rules are about the 
>> position of the newline
>>  a. Line Length 120
>>  b. Operator Wrap
>>  c. One Statement Per Line
>>  d. Multiple Variable Declarations
>> 
>> 
>> Step 4: (674 files, 6730 lines (with Step 1, Step 2 and Step 3))
>> 
>> 1. Set below rules' severity to error in Checkstyle
>>  a. Array Type Style
>>  b. Upper Ell
>>  c. Modifier Order
>>  d. Empty Line Separator
>>  e. Separator Wrap
>>  f. Overload Methods Declaration Order
>>  g. Variable Declaration Usage Distance
>>  i. One Top Level Class
>> 
>> 
>> Step 5:
>> 
>> 1. Add java doc check to CheckStyle and set severity to warning.
>> 
>> 2. Fix java doc warning gradually
>> 
>> 
>>> 2022年4月28日 00:39,morrysnow  写道:
>>> 
>>> Hi, devs
>>> 
>>> I summarized all the rules I wanted to add. I examined the number of lines 
>>> of code affected by all the rules and categorized them by importance.
>>> 
>>> The link is here: 
>>> https://github.com/apache/incubator-doris/issues/8985#issuecomment-214162
>>>  
>>> 
>>> 
>>> I would like to be able to submit them in multiple patches. The principle 
>>> of patch splitting is as follows. Those with low impact and high importance 
>>> are submitted first and sets the severity to Error. The Javadoc commits 
>>> last and sets the severity to Warning.
>>

Re: Add more rules to checkstyle.xml in fe

2022-04-28 Thread morrysnow
Hi, devs

I want to add Step 0 before all steps.

Step 0:

Add checkstyle action to github action to check pr style. This action checks 
incremental code and adds comment to pr automatically.
Action: https://github.com/dbelyaev/action-checkstyle 

Demo: https://github.com/morrySnow/test_checkstyle/pull/5 


> 2022年4月29日 13:40,morrysnow  写道:
> 
> Hi, mingyu
> 
> 1. Check license is needed for local check.
> 2. Add a rule to forbidden static import is ok for me
> 
>> 2022年4月28日 21:34,陈明雨  写道:
>> 
>> That would be great!
>> Some suggestion about the rules:
>> 
>> 
>>> d. license header check
>> For now, we use skywalking-eyes to check license header, is this rule still 
>> needed?
>> 
>> 
>>> adjust the order of import to "static, org.apache.doris, third party, java"
>> Better not using static import
>> 
>> 
>> 
>> 
>> --
>> 
>> 此致!Best Regards
>> 陈明雨 Mingyu Chen
>> 
>> Email:
>> chenmin...@apache.org
>> 
>> 
>> 
>> 
>> 
>> 在 2022-04-28 17:14:34,"morrysnow"  写道:
>>> Hi, devs,
>>> 
>>> I split adding java check style into 5 steps. I’m willing to get feedback 
>>> about it.
>>> 
>>> CheckStyle and Spotless will modify about 10k lines.
>>> 
>>> Step 1:
>>> 
>>> 1. Add Spotless plugin and do some simple format(1198 files changed, 3205 
>>> insertions(+), 4350 deletions(-))
>>> a. end with new line
>>> b. trim trailing whitespace
>>> c. replace tab indent with 4 spaces indent
>>> d. license header check
>>> e. adjust the order of import to "static, org.apache.doris, third 
>>> party, java"
>>> f. remove unused imports
>>> g. make sure all files are encoding by utf-8
>>> h. make sure all files line ending by LF
>>> 
>>> 2. Add checkstyle rules but set default severity to warning to avoid 
>>> compile failed.
>>> 
>>> 3. set some rules' severity to error in Checkstyle since no lines in doris 
>>> break these rules(0 files, 0 lines)
>>> a. Merge conflicts unresolved
>>> b. Avoid using corresponding octal or Unicode escape
>>> c. Avoid Escaped Unicode Characters
>>> d. No Line Wrap
>>> e. Package Name
>>> f. Type Name
>>> g. Annotation Location
>>> h. Interface Type Parameter 
>>> i. CatchParameterName
>>> j. Pattern Variable Name
>>> k. Record Component Name
>>> l. Record Type Parameter Name
>>> m. Method Type Parameter Name
>>> 
>>> 4. Set below rules severity to error in Checkstyle since these had done by 
>>> splotless(12 files, 177 lines)
>>> a. Redundant Import
>>> b. Custom Import Order
>>> c. Unused Imports
>>> d. Avoid Star Import
>>> e. tab character in file
>>> f. Newline At End Of File
>>> g. Trailing whitespace found
>>> 
>>> 
>>> 
>>> Step 2: (241 files, 962 lines (with Step 1))
>>> 
>>> 1. Set below rules' severity to error in Checkstyle, all rules are import 
>>> to make sure the code is correct
>>> a. Need Braces
>>> b. Equals Hash Code
>>> c. Missing Switch Default
>>> d. Fall Through
>>> e. No Finalizer
>>> 
>>> 
>>> 2. Set below rules' severity to error in Checkstyle, all rules are about 
>>> name
>>> a. Member Name
>>> b. Constant Name
>>> c. Parameter Name
>>> d. LambdaParameterName
>>> e. Local Variable Name
>>> f. Class Type Parameter Name
>>> g. Static Variable Name
>>> h. Method Name
>>> j. Abbreviation As Word In Name
>>> 
>>> 
>>> Step 3: (605 files, 6008 lines (with Step 1 and Step 2))
>>> 
>>> 1. Set below rules' severity to error in Checkstyle, all rules are about 
>>> whitespace
>>> a. Empty Block
>>> b. Left Curly
>>> c. Right Curly
>>> d. White space Around
>>> e. Generic Whitespace
>>> f. Indentation
>>> g. No Whitespace Before Case Default Colon
>>> h. Method Param Pad
>>> i. Whitespace
>>> j. Paren Pad
>>> k. Extra newline
>>> l. Extra newline at end of block
>>> m. Empty Statement
>>> n. Empty Catch Block
>>> o. Comments Indentation
>>> 
>>> 2. Set below rules' severity to error in Checkstyle, all rules are about 
>>> the position of the newline
>>> a. Line Length 120
>>> b. Operator Wrap
>>> c. One Statement Per Line
>>> d. Multiple Variable Declarations
>>> 
>>> 
>>> Step 4: (674 files, 6730 lines (with Step 1, Step 2 and Step 3))
>>> 
>>> 1. Set below rules' severity to error in Checkstyle
>>> a. Array Type Style
>>> b. Upper Ell
>>> c. Modifier Order
>>> d. Empty Line Separator
>>> e. Separator Wrap
>>> f. Overload Methods Declaration Order
>>> g. Variable Declaration Usage Distance
>>> i. One Top Level Class
>>> 
>>> 
>>> Step 5:
>>> 
>>> 1. Add java doc check to CheckStyle and set severity to warning.
>>> 
>>> 2. Fix java doc warning gradually
>>> 
>>> 
 2022年4月28日 00:39,morrysnow  写道:
 
 Hi, devs
 
 I summarized all the rules I wanted to add. I examined the number of lines 
 of cod