You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by sr...@apache.org on 2016/08/04 20:43:09 UTC

spark git commit: [SPARK-16877][BUILD] Add rules for preventing to use Java annotations (Deprecated and Override)

Repository: spark
Updated Branches:
  refs/heads/master 462784ffa -> 1d781572e


[SPARK-16877][BUILD] Add rules for preventing to use Java annotations (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 <gu...@gmail.com>

Closes #14490 from HyukjinKwon/SPARK-16877.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/1d781572
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/1d781572
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/1d781572

Branch: refs/heads/master
Commit: 1d781572e832058e2ef54bccd76ef71bc1fd548c
Parents: 462784f
Author: hyukjinkwon <gu...@gmail.com>
Authored: Thu Aug 4 21:43:05 2016 +0100
Committer: Sean Owen <so...@cloudera.com>
Committed: Thu Aug 4 21:43:05 2016 +0100

----------------------------------------------------------------------
 scalastyle-config.xml | 8 ++++++++
 1 file changed, 8 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/1d781572/scalastyle-config.xml
----------------------------------------------------------------------
diff --git a/scalastyle-config.xml b/scalastyle-config.xml
index 9a35183..7fe0697 100644
--- a/scalastyle-config.xml
+++ b/scalastyle-config.xml
@@ -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        -->
   <!-- ================================================================================ -->


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org