[SPARK-2497] Included checks for module symbols too.

[ScrapCodes]

No description provided.

[SparkQA]

QA tests have started for PR 1463. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16777/consoleFull

[SparkQA]

QA results for PR 1463:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16777/consoleFull

[pwendell]

@ScrapCodes could you explain a bit more how this fixes SPARK-2497. If I look at the original false-positive the issue reported was not with a companion class. It was actually in a method of the object itself. Also the method was annotated, not the object.

Source: https://github.com/apache/spark/pull/886/files#diff-0f907b47af6261abe00eb31097a9493bR41
Error:
[error]  * method calculate(Double,Double)Double in object org.apache.spark.mllib.tree.impurity.Gini's type has changed; was (Double,Double)Double, is now: (Array[Double],Double)Double
[error]    filter with: ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.mllib.tree.impurity.Gini.calculate")

[ScrapCodes]

@pwendell The reason I did this is, if you compile a file A.scala with contents

@SomeAnnotation
object A

it will produce two class files A.class and A$.class. And I don't understand why scala reflection does not detect the annotation on A.class, it only detects it on A$.class. This was the reason for doing this change.

The impact of this change is that, when we see annotation on a class we assume it is also present on its companion.

I am spending a little more time on this maybe something better can be done.

[ScrapCodes]

Also according to current code flow we don't check methods if we spot an annotation on a class. I can remove this restriction if you are okay with it.

[SparkQA]

QA tests have started for PR 1463. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16910/consoleFull

[SparkQA]

QA results for PR 1463:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16910/consoleFull

[pwendell]

In your example, the reason why there is no annotation on the A class is that it the class is distinct from the companion object. You only created the companion object in your example. I think that in this case scala will create an empty class. What happens if you compile this:

@SomeAnnotation
object A

@SomeAnnotation
class A

Will you see the annotation on both?

I this current approach will cause false negatives in the following situation:

// This is not a developer API
class A

// This is a developer API
@DeveloperAPI
object A

In this case changes to the A class would be ignored, incorrectly.

[ScrapCodes]

I changed the patch to first check for presence of annotation on class and then on object using separate symbol for both. Previously I was doing that because I did not knew that I could use moduleSymbols for objects and classSymbols for classes.

[SparkQA]

QA tests have started for PR 1463. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17441/consoleFull

[SparkQA]

QA results for PR 1463:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17441/consoleFull

[pwendell]

Hey @ScrapCodes sorry I was misunderstanding what you were saying. You are just trying to reflect on the sybmol as both a class and a module.

[pwendell]

Thanks @ScrapCodes I think I understand this all now. I'll merge this.