Skip to content

Commit

Permalink
[SPARK-16877][BUILD] Add rules for preventing to use Java annotations…
Browse files Browse the repository at this point in the history
… (Deprecated and Override)

## What changes were proposed in this pull request?

This PR adds both rules for preventing to use `Deprecated` and `Override`.

- Java's `Override`
  It seems Scala compiler just ignores this. Apparently, `override` modifier is only mandatory for " that override some other **concrete member definition** in a parent class" but not for for **incomplete member definition** (such as ones from trait or abstract), see (http://www.scala-lang.org/files/archive/spec/2.11/05-classes-and-objects.html#override)

  For a simple example,

  - Normal class - needs `override` modifier

  ```bash
  scala> class A { def say = {}}
  defined class A

  scala> class B extends A { def say = {}}
  <console>:8: error: overriding method say in class A of type => Unit;
   method say needs `override' modifier
         class B extends A { def say = {}}
                                 ^
  ```

  - Trait - does not need `override` modifier

  ```bash
  scala> trait A { def say }
  defined trait A

  scala> class B extends A { def say = {}}
  defined class B
  ```

  To cut this short, this case below is possible,

  ```bash
  scala> class B extends A {
       |    Override
       |    def say = {}
       | }
  defined class B
  ```
  we can write `Override` annotation (meaning nothing) which might confuse engineers that Java's annotation is working fine. It might be great if we prevent those potential confusion.

- Java's `Deprecated`
  When `Deprecated` is used,  it seems Scala compiler recognises this correctly but it seems we use Scala one `deprecated` across codebase.

## How was this patch tested?

Manually tested, by inserting both `Override` and `Deprecated`. This will shows the error messages as below:

```bash
Scalastyle checks failed at following occurrences:
[error] ... : deprecated should be used instead of java.lang.Deprecated.
```

```basg
Scalastyle checks failed at following occurrences:
[error] ... : override modifier should be used instead of java.lang.Override.
```

Author: hyukjinkwon <[email protected]>

Closes apache#14490 from HyukjinKwon/SPARK-16877.
  • Loading branch information
HyukjinKwon authored and srowen committed Aug 4, 2016
1 parent 462784f commit 1d78157
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions scalastyle-config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,14 @@ This file is divided into 3 sections:
<customMessage>Omit braces in case clauses.</customMessage>
</check>

<!-- SPARK-16877: Avoid Java annotations -->
<check customId="OverrideJavaCase" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true">
<parameters><parameter name="regex">^Override$</parameter></parameters>
<customMessage>override modifier should be used instead of @java.lang.Override.</customMessage>
</check>

<check level="error" class="org.scalastyle.scalariform.DeprecatedJavaChecker" enabled="true"></check>

<!-- ================================================================================ -->
<!-- rules we'd like to enforce, but haven't cleaned up the codebase yet -->
<!-- ================================================================================ -->
Expand Down

0 comments on commit 1d78157

Please sign in to comment.