Skip to content

Commit d7304d3

Browse files
CRogersCRogers
authored
Optimise imports on save (#906)
Ensure imports are optimised on save, as well as formatting.
1 parent f7cd2bf commit d7304d3

File tree

5 files changed

+101
-77
lines changed

5 files changed

+101
-77
lines changed

‎.palantir/revapi.yml‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
versionOverrides: {}
21
acceptedBreaks:
32
"0.3.9":
4-
com.palantir.javaformat:palantir-java-format-spi: []
53
com.palantir.javaformat:gradle-palantir-java-format:
6-
- code: "java.class.removed"
7-
old: "class com.palantir.javaformat.gradle.ConfigureExternalDependenciesXml"
8-
new: null
9-
justification: "nobody used this directly"
10-
- code: "java.class.removed"
11-
old: "class com.palantir.javaformat.gradle.ConfigurePalantirJavaFormatXml"
12-
new: null
13-
justification: "nobody used this directly"
14-
- code: "java.class.removed"
15-
old: "class com.palantir.javaformat.gradle.UpdateIntellijXmlTask"
16-
new: null
17-
justification: "nobody used this directly"
4+
- code: "java.class.removed"
5+
old: "class com.palantir.javaformat.gradle.ConfigureExternalDependenciesXml"
6+
justification: "nobody used this directly"
7+
- code: "java.class.removed"
8+
old: "class com.palantir.javaformat.gradle.ConfigurePalantirJavaFormatXml"
9+
justification: "nobody used this directly"
10+
- code: "java.class.removed"
11+
old: "class com.palantir.javaformat.gradle.UpdateIntellijXmlTask"
12+
justification: "nobody used this directly"
13+
"2.33.0":
14+
com.palantir.javaformat:gradle-palantir-java-format:
15+
- code: "java.method.visibilityReduced"
16+
old: "method void com.palantir.javaformat.gradle.ConfigureJavaFormatterXml::configureFormatOnSave(groovy.util.Node)"
17+
new: "method void com.palantir.javaformat.gradle.ConfigureJavaFormatterXml::configureFormatOnSave(groovy.util.Node)"
18+
justification: "Not public api, not split over multiple jars"

‎changelog/@unreleased/pr-906.v2.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
type: fix
2+
fix:
3+
description: Ensure imports are optimised on save, as well as formatting.
4+
links:
5+
- https://github.com/palantir/palantir-java-format/pull/906

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,29 @@ class ConfigureJavaFormatterXml {
3535
matchOrCreateChild(externalDependencies, 'plugin', [id: 'palantir-java-format'])
3636
}
3737

38-
static void configureFormatOnSave(Node rootNode) {
39-
def formatOnSaveOptions = matchOrCreateChild(rootNode, 'component', [name: 'FormatOnSaveOptions'])
38+
static void configureWorkspaceXml(Node rootNode) {
39+
configureFormatOnSave(rootNode)
40+
configureOptimizeOnSave(rootNode)
41+
}
42+
43+
private static void configureFormatOnSave(Node rootNode) {
44+
configureOnSaveAction(matchOrCreateChild(rootNode, 'component', [name: 'FormatOnSaveOptions']))
45+
}
4046

47+
private static void configureOptimizeOnSave(Node rootNode) {
48+
configureOnSaveAction(matchOrCreateChild(rootNode, 'component', [name: 'OptimizeOnSaveOptions']))
49+
}
4150

42-
def myRunOnSave = matchOrCreateChild(formatOnSaveOptions, 'option', [name: 'myRunOnSave'])
51+
private static void configureOnSaveAction(Node onSaveOptions) {
52+
def myRunOnSave = matchOrCreateChild(onSaveOptions, 'option', [name: 'myRunOnSave'])
4353

4454
def myRunOnSaveAlreadySet = Optional.ofNullable(myRunOnSave.attribute('value'))
4555
.map(Boolean::parseBoolean)
4656
.orElse(false)
4757

4858
myRunOnSave.attributes().put('value', 'true')
4959

50-
def myAllFileTypesSelectedAlreadySet = matchChild(formatOnSaveOptions, 'option', [name: 'myAllFileTypesSelected'])
60+
def myAllFileTypesSelectedAlreadySet = matchChild(onSaveOptions, 'option', [name: 'myAllFileTypesSelected'])
5161
.map { Boolean.parseBoolean(it.attribute('value')) }
5262
.orElse(true)
5363

@@ -59,10 +69,10 @@ class ConfigureJavaFormatterXml {
5969
}
6070

6171
// Otherwise we setup intellij to not format all files...
62-
matchOrCreateChild(formatOnSaveOptions, 'option', [name: 'myAllFileTypesSelected']).attributes().put('value', 'false')
72+
matchOrCreateChild(onSaveOptions, 'option', [name: 'myAllFileTypesSelected']).attributes().put('value', 'false')
6373

6474
// ...but ensure java is formatted
65-
def mySelectedFileTypes = matchOrCreateChild(formatOnSaveOptions, 'option', [name: 'mySelectedFileTypes'])
75+
def mySelectedFileTypes = matchOrCreateChild(onSaveOptions, 'option', [name: 'mySelectedFileTypes'])
6676
def set = matchOrCreateChild(mySelectedFileTypes, 'set')
6777
matchOrCreateChild(set, 'option', [value: 'JAVA'])
6878
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ private static void configureLegacyIdea(Project project, Configuration implConfi
6868
});
6969

7070
ideaModel.getWorkspace().getIws().withXml(xmlProvider -> {
71-
ConfigureJavaFormatterXml.configureFormatOnSave(xmlProvider.asNode());
71+
ConfigureJavaFormatterXml.configureWorkspaceXml(xmlProvider.asNode());
7272
});
7373
}
7474

@@ -91,15 +91,15 @@ private static void configureIntelliJImport(Project project, Configuration implC
9191
project.file(".idea/externalDependencies.xml"),
9292
node -> ConfigureJavaFormatterXml.configureExternalDependencies(node));
9393
createOrUpdateIdeaXmlFile(
94-
project.file(".idea/workspace.xml"), node -> ConfigureJavaFormatterXml.configureFormatOnSave(node));
94+
project.file(".idea/workspace.xml"), node -> ConfigureJavaFormatterXml.configureWorkspaceXml(node));
9595

9696
// Still configure legacy idea if using intellij import
9797
updateIdeaXmlFileIfExists(project.file(project.getName() + ".ipr"), node -> {
9898
ConfigureJavaFormatterXml.configureJavaFormat(node, uris);
9999
ConfigureJavaFormatterXml.configureExternalDependencies(node);
100100
});
101101
updateIdeaXmlFileIfExists(project.file(project.getName() + ".iws"), node -> {
102-
ConfigureJavaFormatterXml.configureFormatOnSave(node);
102+
ConfigureJavaFormatterXml.configureWorkspaceXml(node);
103103
});
104104
});
105105
}

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

Lines changed: 62 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.palantir.javaformat.gradle
1818

1919
import spock.lang.Specification
20+
import spock.lang.Unroll
2021

2122
class ConfigureJavaFormatterXmlTest extends Specification {
2223

@@ -60,6 +61,7 @@ class ConfigureJavaFormatterXmlTest extends Specification {
6061
</component>
6162
</root>
6263
""".stripIndent()
64+
public static final ArrayList<String> ACTIONS_ON_SAVE = ['Format', 'Optimize']
6365

6466
void testConfigure_missingEntireBlock_added() {
6567
def node = new XmlParser().parseText(MISSING_ENTIRE_BLOCK)
@@ -104,41 +106,43 @@ class ConfigureJavaFormatterXmlTest extends Specification {
104106
xmlToString(node) == EXPECTED
105107
}
106108

107-
void 'adds FormatOnSave block where none exists'() {
109+
@Unroll
110+
void 'adds #action OnSave block where none exists'(action) {
108111
// language=xml
109112
def node = new XmlParser().parseText '''
110113
<root>
111114
</root>
112115
'''.stripIndent(true)
113116

114117
when:
115-
ConfigureJavaFormatterXml.configureFormatOnSave(node)
116-
def newXml = xmlToString(node).strip()
118+
ConfigureJavaFormatterXml.configureWorkspaceXml(node)
119+
120+
def newXml = xmlSubcomponentToString(node, "${action}OnSaveOptions").strip()
117121

118122
then:
119-
// language=xml
120-
def expected = '''
121-
<root>
122-
<component name="FormatOnSaveOptions">
123-
<option name="myRunOnSave" value="true"/>
124-
<option name="myAllFileTypesSelected" value="false"/>
125-
<option name="mySelectedFileTypes">
126-
<set>
127-
<option value="JAVA"/>
128-
</set>
129-
</option>
130-
</component>
131-
</root>
132-
'''.stripIndent(true).strip()
123+
def expected = """
124+
<component name="${action}OnSaveOptions">
125+
<option name="myRunOnSave" value="true"/>
126+
<option name="myAllFileTypesSelected" value="false"/>
127+
<option name="mySelectedFileTypes">
128+
<set>
129+
<option value="JAVA"/>
130+
</set>
131+
</option>
132+
</component>
133+
""".stripIndent(true).strip()
133134

134135
newXml == expected
136+
137+
where:
138+
action << ACTIONS_ON_SAVE
135139
}
136140

137-
void 'adds Java to existing FormatOnSave block'() {
138-
// language=xml
139-
def node = new XmlParser().parseText '''
141+
@Unroll
142+
void 'adds Java to existing #action OnSave block'(action) {
143+
def node = new XmlParser().parseText """
140144
<root>
141-
<component name="FormatOnSaveOptions">
145+
<component name="${action}OnSaveOptions">
142146
<option name="myRunOnSave" value="true"/>
143147
<option name="myAllFileTypesSelected" value="false"/>
144148
<option name="mySelectedFileTypes">
@@ -148,58 +152,62 @@ class ConfigureJavaFormatterXmlTest extends Specification {
148152
</option>
149153
</component>
150154
</root>
151-
'''.stripIndent(true)
155+
""".stripIndent(true)
152156

153157
when:
154-
ConfigureJavaFormatterXml.configureFormatOnSave(node)
155-
def newXml = xmlToString(node).strip()
158+
ConfigureJavaFormatterXml.configureWorkspaceXml(node)
159+
def newXml = xmlSubcomponentToString(node, "${action}OnSaveOptions")
156160

157161
then:
158-
// language=xml
159-
def expected = '''
160-
<root>
161-
<component name="FormatOnSaveOptions">
162-
<option name="myRunOnSave" value="true"/>
163-
<option name="myAllFileTypesSelected" value="false"/>
164-
<option name="mySelectedFileTypes">
165-
<set>
166-
<option value="Go"/>
167-
<option value="JAVA"/>
168-
</set>
169-
</option>
170-
</component>
171-
</root>
172-
'''.stripIndent(true).strip()
162+
def expected = """
163+
<component name="${action}OnSaveOptions">
164+
<option name="myRunOnSave" value="true"/>
165+
<option name="myAllFileTypesSelected" value="false"/>
166+
<option name="mySelectedFileTypes">
167+
<set>
168+
<option value="Go"/>
169+
<option value="JAVA"/>
170+
</set>
171+
</option>
172+
</component>
173+
""".stripIndent(true).strip()
173174

174175
newXml == expected
176+
177+
where:
178+
action << ACTIONS_ON_SAVE
175179
}
176180

177-
void 'if all file types are already formatted on save, dont change anything'() {
178-
// language=xml
179-
def node = new XmlParser().parseText '''
181+
@Unroll
182+
void 'if all file types are already configured to #action on save, dont change anything'() {
183+
def node = new XmlParser().parseText """
180184
<root>
181-
<component name="FormatOnSaveOptions">
185+
<component name="${action}OnSaveOptions">
182186
<option name="myRunOnSave" value="true"/>
183187
<!-- if myAllFileTypesSelected does not exist, it defaults to true -->
184188
</component>
185189
</root>
186-
'''.stripIndent(true)
190+
""".stripIndent(true)
187191

188192
when:
189-
ConfigureJavaFormatterXml.configureFormatOnSave(node)
190-
def newXml = xmlToString(node).strip()
193+
ConfigureJavaFormatterXml.configureWorkspaceXml(node)
194+
def newXml = xmlSubcomponentToString(node, "${action}OnSaveOptions").strip()
191195

192196
then:
193-
// language=xml
194-
def expected = '''
195-
<root>
196-
<component name="FormatOnSaveOptions">
197-
<option name="myRunOnSave" value="true"/>
198-
</component>
199-
</root>
200-
'''.stripIndent(true).strip()
197+
def expected = """
198+
<component name="${action}OnSaveOptions">
199+
<option name="myRunOnSave" value="true"/>
200+
</component>
201+
""".stripIndent(true).strip()
201202

202203
newXml == expected
204+
205+
where:
206+
action << ACTIONS_ON_SAVE
207+
}
208+
209+
private static String xmlSubcomponentToString(Node node, String name) {
210+
xmlToString(node.children().find { it.@name == name }).strip()
203211
}
204212

205213
static String xmlToString(Node node) {

0 commit comments

Comments
 (0)