Add ModifiersOrderOperation (java:S1124)

astra-core/src/main/java/org/alfasoftware/astra/core/refactoring/operations/sonar/s1124/ModifiersOrderOperation.java

@@ -0,0 +1,158 @@
+package org.alfasoftware.astra.core.refactoring.operations.sonar.s1124;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.alfasoftware.astra.core.utils.ASTOperation;
+import org.alfasoftware.astra.core.utils.AstraUtils;
+import org.eclipse.jdt.core.dom.AST;
+import org.eclipse.jdt.core.dom.ASTNode;
+import org.eclipse.jdt.core.dom.BodyDeclaration;
+import org.eclipse.jdt.core.dom.ChildListPropertyDescriptor;
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.dom.IExtendedModifier;
+import org.eclipse.jdt.core.dom.Modifier;
+import org.eclipse.jdt.core.dom.StructuralPropertyDescriptor;
+import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
+import org.eclipse.jdt.core.dom.rewrite.ListRewrite;
+import org.eclipse.jface.text.BadLocationException;
+import org.eclipse.text.edits.MalformedTreeException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Refactoring operation implementing SonarQube rule java:S1124
+ * "Modifiers should be declared in the correct order".
+ *
+ * <p>The Java Language Specification recommends the following canonical modifier order:
+ * <ol>
+ *   <li>Annotations</li>
+ *   <li>{@code public}</li>
+ *   <li>{@code protected}</li>
+ *   <li>{@code private}</li>
+ *   <li>{@code abstract}</li>
+ *   <li>{@code default}</li>
+ *   <li>{@code static}</li>
+ *   <li>{@code final}</li>
+ *   <li>{@code transient}</li>
+ *   <li>{@code volatile}</li>
+ *   <li>{@code synchronized}</li>
+ *   <li>{@code native}</li>
+ *   <li>{@code strictfp}</li>
+ * </ol>
+ *
+ * <p>When keyword modifiers appear out of this order on any body declaration (type, method, field,
+ * etc.) they are reordered in-place. Annotations are left at their original positions.
+ *
+ * <p>Examples:
+ * <ul>
+ *   <li>{@code static public void foo()}      &rarr; {@code public static void foo()}</li>
+ *   <li>{@code final static int X = 1}        &rarr; {@code static final int X = 1}</li>
+ *   <li>{@code final public static String S}  &rarr; {@code public static final String S}</li>
+ *   <li>{@code synchronized static void bar()} &rarr; {@code static synchronized void bar()}</li>
+ * </ul>
+ *
+ * <p>Mirrors the detection logic of SonarJava's {@code ModifiersOrderCheck}.
+ */
+public class ModifiersOrderOperation implements ASTOperation {
+
+  private static final Logger log = LoggerFactory.getLogger(ModifiersOrderOperation.class);
+
+  /**
+   * Canonical modifier order per the Java Language Specification (JLS 17).
+   * Modifiers not present in this list are placed at the end in their original relative order.
+   */
+  static final List<Modifier.ModifierKeyword> MODIFIER_ORDER = List.of(
+      Modifier.ModifierKeyword.PUBLIC_KEYWORD,
+      Modifier.ModifierKeyword.PROTECTED_KEYWORD,
+      Modifier.ModifierKeyword.PRIVATE_KEYWORD,
+      Modifier.ModifierKeyword.ABSTRACT_KEYWORD,
+      Modifier.ModifierKeyword.DEFAULT_KEYWORD,
+      Modifier.ModifierKeyword.STATIC_KEYWORD,
+      Modifier.ModifierKeyword.FINAL_KEYWORD,
+      Modifier.ModifierKeyword.TRANSIENT_KEYWORD,
+      Modifier.ModifierKeyword.VOLATILE_KEYWORD,
+      Modifier.ModifierKeyword.SYNCHRONIZED_KEYWORD,
+      Modifier.ModifierKeyword.NATIVE_KEYWORD,
+      Modifier.ModifierKeyword.STRICTFP_KEYWORD
+  );
+
+  @Override
+  public void run(CompilationUnit compilationUnit, ASTNode node, ASTRewrite rewriter)
+      throws IOException, MalformedTreeException, BadLocationException {
+
+    if (!(node instanceof BodyDeclaration)) {
+      return;
+    }
+
+    BodyDeclaration bodyDecl = (BodyDeclaration) node;
+
+    @SuppressWarnings("unchecked")
+    List<IExtendedModifier> allModifiers = bodyDecl.modifiers();
+
+    List<Modifier> keywordModifiers = allModifiers.stream()
+        .filter(m -> m instanceof Modifier)
+        .map(m -> (Modifier) m)
+        .collect(Collectors.toList());
+
+    if (keywordModifiers.size() < 2) {
+      return;
+    }
+
+    List<Modifier.ModifierKeyword> currentOrder = keywordModifiers.stream()
+        .map(Modifier::getKeyword)
+        .collect(Collectors.toList());
+
+    List<Modifier.ModifierKeyword> sortedOrder = new ArrayList<>(currentOrder);
+    sortedOrder.sort(Comparator.comparingInt(k -> {
+      int idx = MODIFIER_ORDER.indexOf(k);
+      return idx == -1 ? MODIFIER_ORDER.size() : idx;
+    }));
+
+    if (currentOrder.equals(sortedOrder)) {
+      return;
+    }
+
+    log.info("Reordering modifiers in [{}]: {} -> {}",
+        AstraUtils.getNameForCompilationUnit(compilationUnit),
+        currentOrder.stream().map(k -> k.toString()).collect(Collectors.joining(" ")),
+        sortedOrder.stream().map(k -> k.toString()).collect(Collectors.joining(" ")));
+
+    ChildListPropertyDescriptor modifiersProperty = findModifiersProperty(bodyDecl);
+    if (modifiersProperty == null) {
+      return;
+    }
+
+    AST ast = compilationUnit.getAST();
+    ListRewrite listRewrite = rewriter.getListRewrite(bodyDecl, modifiersProperty);
+
+    for (int i = 0; i < keywordModifiers.size(); i++) {
+      Modifier.ModifierKeyword target = sortedOrder.get(i);
+      if (!currentOrder.get(i).equals(target)) {
+        listRewrite.replace(keywordModifiers.get(i), ast.newModifier(target), null);
+      }
+    }
+  }
+
+  /**
+   * Finds the "modifiers" ChildListPropertyDescriptor for the given node by inspecting
+   * its structural properties. This avoids hard-coding per-subclass property constants
+   * (MethodDeclaration.MODIFIERS2_PROPERTY, FieldDeclaration.MODIFIERS2_PROPERTY, etc.)
+   * which are distinct objects even though they share the same semantics.
+   */
+  @SuppressWarnings("rawtypes")
+  private static ChildListPropertyDescriptor findModifiersProperty(ASTNode node) {
+    for (Object prop : node.structuralPropertiesForType()) {
+      if (prop instanceof ChildListPropertyDescriptor) {
+        ChildListPropertyDescriptor clpd = (ChildListPropertyDescriptor) prop;
+        if ("modifiers".equals(clpd.getId())) {
+          return clpd;
+        }
+      }
+    }
+    return null;
+  }
+}

astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/operations/sonar/s1124/ModifiersOrderExample.java

@@ -0,0 +1,29 @@
+package org.alfasoftware.astra.core.refactoring.operations.sonar.s1124;
+
+public class ModifiersOrderExample {
+
+  // static before public — wrong order on a method
+  static public void staticBeforePublic() {}
+
+  // final before static — wrong order on a field
+  final static int FINAL_STATIC = 1;
+
+  // all three wrong: final before public before static
+  final public static String ALL_THREE_WRONG = "x";
+
+  // synchronized before static — wrong order on a method
+  synchronized static void synchronizedStatic() {}
+
+  // native before static — wrong order (native method has no body)
+  native static void nativeStatic();
+
+  // volatile before transient — wrong order on a field
+  volatile transient int volatileTransient;
+
+  // nested class: abstract static public — wrong order (public should lead, then abstract, then static)
+  abstract static public class WrongNestedClass {
+
+    // synchronized before static inside nested class — wrong order
+    synchronized static void nestedSyncStatic() {}
+  }
+}

astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/operations/sonar/s1124/ModifiersOrderExampleAfter.java

@@ -0,0 +1,29 @@
+package org.alfasoftware.astra.core.refactoring.operations.sonar.s1124;
+
+public class ModifiersOrderExampleAfter {
+
+  // static before public — wrong order on a method
+  public static void staticBeforePublic() {}
+
+  // final before static — wrong order on a field
+  static final int FINAL_STATIC = 1;
+
+  // all three wrong: final before public before static
+  public static final String ALL_THREE_WRONG = "x";
+
+  // synchronized before static — wrong order on a method
+  static synchronized void synchronizedStatic() {}
+
+  // native before static — wrong order (native method has no body)
+  static native void nativeStatic();
+
+  // volatile before transient — wrong order on a field
+  transient volatile int volatileTransient;
+
+  // nested class: abstract static public — wrong order (public should lead, then abstract, then static)
+  public abstract static class WrongNestedClass {
+
+    // synchronized before static inside nested class — wrong order
+    static synchronized void nestedSyncStatic() {}
+  }
+}

astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/operations/sonar/s1124/ModifiersOrderNoopExample.java

@@ -0,0 +1,47 @@
+package org.alfasoftware.astra.core.refactoring.operations.sonar.s1124;
+
+public class ModifiersOrderNoopExample {
+
+  // public static — already correct
+  public static void publicStatic() {}
+
+  // public static final — already correct
+  public static final int PUBLIC_STATIC_FINAL = 1;
+
+  // private static final — already correct
+  private static final String PRIVATE_STATIC_FINAL = "x";
+
+  // static synchronized — already correct
+  static synchronized void staticSynchronized() {}
+
+  // transient volatile — already correct (transient < volatile in JLS order)
+  transient volatile int transientVolatile;
+
+  // public final — already correct
+  public final void publicFinal() {}
+
+  // protected static — already correct
+  protected static void protectedStatic() {}
+
+  // single modifier: cannot be out of order
+  public void singlePublic() {}
+
+  // single modifier
+  static void singleStatic() {}
+
+  // single modifier
+  final int singleFinal = 0;
+
+  // no modifier: nothing to reorder
+  int noModifier;
+
+  // nested class: public abstract static — already correct
+  public abstract static class CorrectNestedClass {
+
+    // static synchronized inside nested class — already correct
+    static synchronized void nestedStaticSync() {}
+
+    // public static final — already correct
+    public static final String NESTED_CONST = "y";
+  }
+}

astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/operations/sonar/s1124/ModifiersOrderNoopExampleAfter.java

@@ -0,0 +1,47 @@
+package org.alfasoftware.astra.core.refactoring.operations.sonar.s1124;
+
+public class ModifiersOrderNoopExampleAfter {
+
+  // public static — already correct
+  public static void publicStatic() {}
+
+  // public static final — already correct
+  public static final int PUBLIC_STATIC_FINAL = 1;
+
+  // private static final — already correct
+  private static final String PRIVATE_STATIC_FINAL = "x";
+
+  // static synchronized — already correct
+  static synchronized void staticSynchronized() {}
+
+  // transient volatile — already correct (transient < volatile in JLS order)
+  transient volatile int transientVolatile;
+
+  // public final — already correct
+  public final void publicFinal() {}
+
+  // protected static — already correct
+  protected static void protectedStatic() {}
+
+  // single modifier: cannot be out of order
+  public void singlePublic() {}
+
+  // single modifier
+  static void singleStatic() {}
+
+  // single modifier
+  final int singleFinal = 0;
+
+  // no modifier: nothing to reorder
+  int noModifier;
+
+  // nested class: public abstract static — already correct
+  public abstract static class CorrectNestedClass {
+
+    // static synchronized inside nested class — already correct
+    static synchronized void nestedStaticSync() {}
+
+    // public static final — already correct
+    public static final String NESTED_CONST = "y";
+  }
+}

astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/operations/sonar/s1124/TestModifiersOrderOperation.java

@@ -0,0 +1,47 @@
+package org.alfasoftware.astra.core.refactoring.operations.sonar.s1124;
+
+import java.util.Set;
+
+import org.alfasoftware.astra.core.refactoring.AbstractRefactorTest;
+import org.junit.Test;
+
+public class TestModifiersOrderOperation extends AbstractRefactorTest {
+
+  private static final Set<ModifiersOrderOperation> OPERATION =
+      Set.of(new ModifiersOrderOperation());
+
+  /**
+   * All cases where keyword modifiers are out of the JLS canonical order and must be reordered:
+   * <ul>
+   *   <li>{@code static public}           &rarr; {@code public static} (method)</li>
+   *   <li>{@code final static}            &rarr; {@code static final} (field)</li>
+   *   <li>{@code final public static}     &rarr; {@code public static final} (field)</li>
+   *   <li>{@code synchronized static}     &rarr; {@code static synchronized} (method)</li>
+   *   <li>{@code abstract static public}  &rarr; {@code public abstract static} (nested class)</li>
+   *   <li>{@code volatile transient}      &rarr; {@code transient volatile} (field)</li>
+   *   <li>{@code native static}           &rarr; {@code static native} (method)</li>
+   *   <li>{@code protected abstract} + {@code static synchronized} inside nested class</li>
+   * </ul>
+   */
+  @Test
+  public void testModifiersReordered() {
+    assertRefactor(ModifiersOrderExample.class, OPERATION);
+  }
+
+  /**
+   * Cases where modifier order is already correct — the operation must make no change:
+   * <ul>
+   *   <li>{@code public static final} field</li>
+   *   <li>{@code public abstract} method</li>
+   *   <li>{@code private static final} field</li>
+   *   <li>{@code protected static} method</li>
+   *   <li>{@code static synchronized} method</li>
+   *   <li>{@code transient volatile} field</li>
+   *   <li>Single-modifier declarations</li>
+   * </ul>
+   */
+  @Test
+  public void testNoopCases() {
+    assertRefactor(ModifiersOrderNoopExample.class, OPERATION);
+  }
+}