Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AnnotationRule: disallow empty line between annotation and annotated target #755

Merged
merged 16 commits into from
Jun 20, 2020

Conversation

AdamMTGreenberg
Copy link
Contributor

@AdamMTGreenberg AdamMTGreenberg commented Jun 1, 2020

This PR fixes #688

In essence, we should not have empty lines between an annotation and the thing that it's "annotating".

My fix adds an extra check in the annotation file that will ensure there is no duplicate spaces between Annotations and the elements that they are annotating. In addition, a suite of new tests have verified this assertion and the auto correct.

@romtsn romtsn changed the title Issue 688 AnnotationRule: disallow empty line between annotation and annotated target Jun 5, 2020
…ent that the annotation is not separated from the object it is annotating
… Eg, this test should result in a failure since the @JvmField annotation is separated from the function by a blank line
…based on above rules for ensuring that we have a stable break with the list of annotations. Next step is to run unit test to make sure this triggers a failure
…hat we can ensure we see a valid break in the tests. Next step is to create a test for the auto correction
…get a solid check for our issue once the autoformat is added to our code
…re we have all the fields input correctly and we remove redundant line breaks
@JvmField @JvmStatic

fun foo() = Unit

// and

@JvmField
@JvmStatic

fun foo() = Unit
val diff = nextLineNumber - lineNumber
// Ensure declaration is not on the same line, there is a line break in between, and it is not an
// annotation we explicitly want to have a line break between
if (lineNumber != nextLineNumber && diff > 1 && !node.text.contains("@file")) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think diff > 1 is sufficient here right?

also not sure about the check for @file presence - aren't you doing this already above with !it.isPartOf(FILE_ANNOTATION_LIST)?

@romtsn romtsn merged commit 4dda69b into pinterest:master Jun 20, 2020
@romtsn
Copy link
Collaborator

romtsn commented Jun 20, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No empty line between annotation and the class/method/property...
2 participants