Fix bug in SuggestedFixes.addModifiers

when you are trying to add a modifier to a method declaration where
there is an annotation on the line before, e.g.

    @nullable
    void foo() {}

Previously this would add the modifier at the beginning of the line, not
indenting properly:

    @nullable
<modifier>    void foo() {}

MOE_MIGRATED_REVID=136207477

check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java

@@ -119,7 +119,7 @@ private static Modifier getTokModifierKind(ErrorProneToken tok) {
 
   /** Add modifiers to the given class, method, or field declaration. */
   @Nullable
-  public static Fix addModifiers(Tree tree, VisitorState state, Modifier... modifiers) {
+  public static SuggestedFix addModifiers(Tree tree, VisitorState state, Modifier... modifiers) {
     ModifiersTree originalModifiers = getModifiers(tree);
     if (originalModifiers == null) {
       return null;
@@ -128,10 +128,19 @@ public static Fix addModifiers(Tree tree, VisitorState state, Modifier... modifi
         Sets.difference(new TreeSet<>(Arrays.asList(modifiers)), originalModifiers.getFlags());
     if (originalModifiers.getFlags().isEmpty()) {
       int pos =
-          state.getEndPosition((JCTree) originalModifiers) != Position.NOPOS
-              ? state.getEndPosition((JCTree) originalModifiers) + 1
+          state.getEndPosition(originalModifiers) != Position.NOPOS
+              ? state.getEndPosition(originalModifiers) + 1
               : ((JCTree) tree).getStartPosition();
-      return SuggestedFix.replace(pos, pos, Joiner.on(' ').join(toAdd) + " ");
+      int base = ((JCTree) tree).getStartPosition();
+      java.util.Optional<Integer> insert =
+          state
+              .getTokensForNode(tree)
+              .stream()
+              .map(token -> token.pos() + base)
+              .filter(thisPos -> thisPos >= pos)
+              .findFirst();
+      int insertPos = insert.orElse(pos); // shouldn't ever be able to get to the else
+      return SuggestedFix.replace(insertPos, insertPos, Joiner.on(' ').join(toAdd) + " ");
     }
     // a map from modifiers to modifier position (or -1 if the modifier is being added)
     // modifiers are sorted in Google Java Style order
@@ -240,7 +249,7 @@ public static String qualifyType(
     fix.addImport(curr.toString());
     return Joiner.on('.').join(names);
   }
-  
+
   public static String qualifyType(
       final TreeMaker make, SuggestedFix.Builder fix, TypeMirror type) {
     return type.accept(
@@ -346,7 +355,7 @@ public static void qualifyDocReference(
   public static Fix addMembers(
       ClassTree classTree, VisitorState state, String firstMember, String... otherMembers) {
     checkNotNull(classTree);
-    int classEndPosition = state.getEndPosition((JCTree) classTree);
+    int classEndPosition = state.getEndPosition(classTree);
     StringBuilder stringBuilder = new StringBuilder();
     for (String memberSnippet : Lists.asList(firstMember, otherMembers)) {
       stringBuilder.append("\n\n").append(memberSnippet);

core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java

@@ -23,13 +23,15 @@
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableMap;
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.BugPattern.Category;
 import com.google.errorprone.BugPattern.SeverityLevel;
 import com.google.errorprone.CompilationTestHelper;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
 import com.google.errorprone.matchers.Description;
@@ -39,6 +41,7 @@
 import com.sun.source.tree.LiteralTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.ReturnTree;
+import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.DocTreePath;
 import com.sun.source.util.DocTreePathScanner;
@@ -74,7 +77,8 @@ enum EditKind {
     summary = "Edits modifiers",
     severity = SeverityLevel.ERROR
   )
-  public static class EditModifiersChecker extends BugChecker implements VariableTreeMatcher {
+  public static class EditModifiersChecker extends BugChecker
+      implements VariableTreeMatcher, MethodTreeMatcher {
 
     static final ImmutableMap<String, Modifier> MODIFIERS_BY_NAME = createModifiersByName();
 
@@ -88,6 +92,15 @@ private static ImmutableMap<String, Modifier> createModifiersByName() {
 
     @Override
     public Description matchVariable(VariableTree tree, VisitorState state) {
+      return editModifiers(tree, state);
+    }
+
+    @Override
+    public Description matchMethod(MethodTree tree, VisitorState state) {
+      return editModifiers(tree, state);
+    }
+
+    private Description editModifiers(Tree tree, VisitorState state) {
       EditModifiers editModifiers =
           ASTHelpers.getAnnotation(
               ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class), EditModifiers.class);
@@ -108,6 +121,34 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
     }
   }
 
+  @Test
+  public void addAtBeginningOfLine() throws IOException {
+    BugCheckerRefactoringTestHelper.newInstance(new EditModifiersChecker(), getClass())
+        .addInputLines(
+            "in/Test.java",
+            "import javax.annotation.Nullable;",
+            String.format("import %s;", EditModifiers.class.getCanonicalName()),
+            "@EditModifiers(value=\"final\", kind=EditModifiers.EditKind.ADD)",
+            "class Test {",
+            "  @Nullable",
+            "  int foo() {",
+            "    return 10;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "out/Test.java",
+            "import javax.annotation.Nullable;",
+            String.format("import %s;", EditModifiers.class.getCanonicalName()),
+            "@EditModifiers(value=\"final\", kind=EditModifiers.EditKind.ADD)",
+            "class Test {",
+            "  @Nullable",
+            "  final int foo() {",
+            "    return 10;",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+
   @Test
   public void addModifiers() {
     CompilationTestHelper.newInstance(EditModifiersChecker.class, getClass())
@@ -186,7 +227,7 @@ public void removeModifiers() {
 
   @BugPattern(
     category = Category.ONE_OFF,
-    
+
     name = "CastReturn",
     severity = SeverityLevel.ERROR,
     summary = "Adds casts to returned expressions"
@@ -210,7 +251,7 @@ public Description matchReturn(ReturnTree tree, VisitorState state) {
 
   @BugPattern(
     category = Category.ONE_OFF,
-    
+
     name = "CastReturn",
     severity = SeverityLevel.ERROR,
     summary = "Adds casts to returned expressions"