chipsalliance / rocket-chip

Rocket Chip Generator
Other
3.18k stars 1.12k forks source link

Add ScalaStyle style checker #255

Open richardxia opened 8 years ago

richardxia commented 8 years ago

What does everyone think about adding the ScalaStyle style checker to rocket-chip in order to enforce a consistent style and to prevent certain classes of errors? I tried running the style checker on the current version of the code, and there are 2818 errors. Here's the counts of all the types of errors:

   1398 Public method must have explicit type
    770 Whitespace at end of line
    238 There should be a space before the plus (+) sign
    176 If block needs braces
     96 There should be a space after the plus (+) sign
     42 File line length exceeds 160 characters
     24 Avoid using null
     14 Boolean expression can be simplified
     12 Number of types declared in the file exceeds 30
     10 Regular expression matched 'println'
      8 Method is longer than 50 lines
      8 File length exceeds 800 lines
      6 Avoid using return
      4 Cyclomatic complexity of 13 exceeds max of 10
      2 The number of parameters should not exceed 8
      2 Number of methods in class exceeds 30
      2 Line contains a tab
      2 Expected token RPAREN but got Token(EQUALS,=,9269,=)
      2 Cyclomatic complexity of 16 exceeds max of 10
      2 Cyclomatic complexity of 11 exceeds max of 10

We can also disable any checks that we don't want.

If you think it's worth adding, I can create a PR along with an update to the Travis settings to automatically run the linter.

mwachs5 commented 8 years ago

I think this could be good for a few of these. My faves (and easy to fix) would be those listed below.

Also maybe tabs only in Makefiles, since I keep screwing that one up.

 42 File line length exceeds 160 characters
 24 Avoid using null
 14 Boolean expression can be simplified 
  8 File length exceeds 800 lines
  6 Avoid using return
  2 The number of parameters should not exceed 8
  2 Number of methods in class exceeds 30
  2 Line contains a tab
  2 Expected token RPAREN but got Token(EQUALS,=,9269,=)      

?? what does this mean (cyclomatic complexity):

      4 Cyclomatic complexity of 13 exceeds max of 10
      2 Cyclomatic complexity of 16 exceeds max of 10
      2 Cyclomatic complexity of 11 exceeds max of 10
richardxia commented 8 years ago

?? what does this mean?

Assuming you meant the Avoid using return, the docs say:

Use of return is not usually necessary in Scala. In fact, use of return can discourage a functional style of programming.

I think it wants you to use the implicit return and avoid early exits from methods. We can turn this off if we don't want it.

aswaterman commented 8 years ago

Definitely should avoid return in Scala; I would keep that one.

On Friday, September 16, 2016, Richard Xia notifications@github.com wrote:

?? what does this mean?

Assuming you meant the Avoid using return, the docs say http://www.scalastyle.org/rules-0.8.0.html#org_scalastyle_scalariform_ReturnChecker :

Use of return is not usually necessary in Scala. In fact, use of return can discourage a functional style of programming.

I think it wants you to use the implicit return and avoid early exits from methods. We can turn this off if we don't want it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/rocket-chip/issues/255#issuecomment-247737964, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-7wijszi5Bvmt-jSKV3ldwkBUbWzZ-ks5qqzZlgaJpZM4J3fhx .

mwachs5 commented 4 years ago

@richardxia i just was cleaning up old issues and still think this would be nice for this code base. I am curious if there is a way to auto-clean some of these. For example "no space before plus sign", "If block needs braces" and so on.

richardxia commented 4 years ago

Oh geez, this is an old ticket. I think I'd defer to @jackkoenig and @azidar. I know in the past couple of years, I've talked to @jackkoenig a bit about using Scalafix instead of Scalastyle, which is more modern and actively maintained, not to mention that there's a mode for automatically fixing issues instead of just yelling at the user to fix it themself.

mwachs5 commented 4 years ago

It is, in fact, the oldes still open ticket :)