Handle actions on save config correctly across IntelliJ versions (#1112)

Ensure Actions on Save is configured properly in IntelliJ <=2023.3.6

changelog/@unreleased/pr-1112.v2.yml

@@ -0,0 +1,5 @@
+type: fix
+fix:
+  description: Ensure Actions on Save is configured properly in IntelliJ <=2023.3.6
+  links:
+  - https://github.com/palantir/palantir-java-format/pull/1112

gradle-palantir-java-format/src/main/groovy/com/palantir/javaformat/gradle/ConfigureJavaFormatterXml.groovy

@@ -49,28 +49,21 @@ class ConfigureJavaFormatterXml {
     }
 
     private static void configureOnSaveAction(Node onSaveOptions) {
-        // If myRunOnSave is set to true, IntelliJ removes it. If it's set to false, we still need to remove it to run
-        // the formatter. So we should just remove it so we do run on save.
-        matchChild(onSaveOptions, 'option', [name: 'myRunOnSave']).ifPresent { myRunOnSave ->
-            onSaveOptions.remove(myRunOnSave)
-        }
-
-        def myAllFilesTypesSelected = matchChild(onSaveOptions, 'option', [name: 'myAllFileTypesSelected'])
-        def myAllFileTypesSelectedAlreadySet = myAllFilesTypesSelected
-                .map { Boolean.parseBoolean(it.attribute('value')) }
-                // If myAllFileTypesSelected is elided then it is disabled by default
-                .orElse(false)
-
-        if (myAllFileTypesSelectedAlreadySet) {
-            // If the user has already configured IntelliJ to format all file types and turned on formatting on save,
-            // we leave the configuration as is as it will format java code, and we don't want to disable formatting
-            // for other file types
-            return
-        }
+        // In IntelliJ <2023.3.7, the myRunOnSave option is default false.
+        // In IntelliJ >=2023.3.7, it is default true, and if it is set to true IntelliJ will remove it from the XML
+        //     the next time the file is saved or the actions on save options window is saved.
+        // So to cover all cases, we always write it out and ensure it is true
+        def myRunOnSave = matchOrCreateChild(onSaveOptions, 'option', [name: 'myRunOnSave'])
+        myRunOnSave.attributes().put('value', 'true')
 
-        myAllFilesTypesSelected.ifPresent { onSaveOptions.remove(it) }
+        // In IntelliJ <2023.3.7, the myAllFileTypesSelected option is default true.
+        // In IntelliJ >=2023.3.7, it is default false, and if it is set to false IntelliJ will remove it from the XML
+        //     the next time the file is saved or the actions on save options window is saved.
+        // People may have manually set this to true, so we respect that setting is so. However, if it doesn't exist,
+        //     we make sure it is explicitly set false to work in all IntelliJ versions.
+        matchOrCreateChild(onSaveOptions, 'option', [name: 'myAllFileTypesSelected'], [value: 'false'])
 
-        // ...but ensure java is formatted
+        // Ensure that is per-file type settings are enabled, we ensure JAVA is part of the list.
         def mySelectedFileTypes = matchOrCreateChild(onSaveOptions, 'option', [name: 'mySelectedFileTypes'])
         def set = matchOrCreateChild(mySelectedFileTypes, 'set')
         matchOrCreateChild(set, 'option', [value: 'JAVA'])

gradle-palantir-java-format/src/test/groovy/com/palantir/javaformat/gradle/ConfigureJavaFormatterXmlTest.groovy

@@ -122,6 +122,8 @@ class ConfigureJavaFormatterXmlTest extends Specification {
         then:
         def expected = """
             <component name="${action}OnSaveOptions">
+              <option name="myRunOnSave" value="true"/>
+              <option name="myAllFileTypesSelected" value="false"/>
               <option name="mySelectedFileTypes">
                 <set>
                   <option value="JAVA"/>
@@ -163,6 +165,8 @@ class ConfigureJavaFormatterXmlTest extends Specification {
                   <option value="JAVA"/>
                 </set>
               </option>
+              <option name="myRunOnSave" value="true"/>
+              <option name="myAllFileTypesSelected" value="false"/>
             </component>
         """.stripIndent(true).strip()
 
@@ -190,6 +194,51 @@ class ConfigureJavaFormatterXmlTest extends Specification {
         def expected = """
             <component name="${action}OnSaveOptions">
               <option name="myAllFileTypesSelected" value="true"/>
+              <option name="myRunOnSave" value="true"/>
+              <option name="mySelectedFileTypes">
+                <set>
+                  <option value="JAVA"/>
+                </set>
+              </option>
+            </component>
+        """.stripIndent(true).strip()
+
+        newXml == expected
+
+        where:
+        action << ACTIONS_ON_SAVE
+    }
+
+    @Unroll
+    def 'if the myRunOnSave for #action on save is explicitly disabled, turn it on'() {
+        def node = new XmlParser().parseText """
+            <root>
+              <component name="${action}OnSaveOptions">
+                <option name="myRunOnSave" value="false"/>
+                <option name="myAllFileTypesSelected" value="false"/>
+                <option name="mySelectedFileTypes">
+                  <set>
+                    <option value="JAVA"/>
+                  </set>
+                </option>
+              </component>
+            </root>
+        """.stripIndent(true)
+
+        when:
+        ConfigureJavaFormatterXml.configureWorkspaceXml(node)
+        def newXml = xmlSubcomponentToString(node, "${action}OnSaveOptions")
+
+        then:
+        def expected = """
+            <component name="${action}OnSaveOptions">
+              <option name="myRunOnSave" value="true"/>
+              <option name="myAllFileTypesSelected" value="false"/>
+              <option name="mySelectedFileTypes">
+                <set>
+                  <option value="JAVA"/>
+                </set>
+              </option>
             </component>
         """.stripIndent(true).strip()