Skip to content

Commit b848048

Browse files
koentsjekoentsje
committed
HBX-3182: Improve the implementation of DefaultJavaPrettyPrinterStrategy
Signed-off-by: Koen Aers <[email protected]>
1 parent 55d4e6a commit b848048

File tree

4 files changed

+48
-134
lines changed

4 files changed

+48
-134
lines changed

‎ant/src/main/java/org/hibernate/tool/ant/JavaFormatterTask.java‎

Lines changed: 39 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@
2222
import java.io.FileInputStream;
2323
import java.io.IOException;
2424
import java.util.ArrayList;
25-
import java.util.Iterator;
2625
import java.util.LinkedList;
2726
import java.util.List;
28-
import java.util.Map;
2927
import java.util.Properties;
3028

3129
import org.apache.tools.ant.BuildException;
@@ -37,77 +35,49 @@
3735

3836
public class JavaFormatterTask extends Task {
3937

40-
private List<FileSet> fileSets = new ArrayList<FileSet>();
38+
private final List<FileSet> fileSets = new ArrayList<FileSet>();
4139
private boolean failOnError;
42-
private File configurationFile;
43-
40+
4441
public void addConfiguredFileSet(FileSet fileSet) {
4542
fileSets.add(fileSet);
4643
}
4744

48-
public void setConfigurationFile(File configurationFile) {
49-
this.configurationFile = configurationFile;
50-
}
51-
5245
private Properties readConfig(File cfgfile) throws IOException {
53-
BufferedInputStream stream = null;
54-
try {
55-
stream = new BufferedInputStream(new FileInputStream(cfgfile));
56-
final Properties settings = new Properties();
57-
settings.load(stream);
58-
return settings;
59-
} catch (IOException e) {
60-
throw e;
61-
} finally {
62-
if (stream != null) {
63-
try {
64-
stream.close();
65-
} catch (IOException e) {
66-
67-
}
68-
}
69-
}
70-
}
46+
try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream(cfgfile))) {
47+
final Properties settings = new Properties();
48+
settings.load(stream);
49+
return settings;
50+
}
51+
}
7152

7253

7354
public void execute() throws BuildException {
7455

75-
Map<Object, Object> settings = null;
76-
if(configurationFile!=null) {
77-
try {
78-
settings = readConfig( configurationFile );
79-
}
80-
catch (IOException e) {
81-
throw new BuildException("Could not read configurationfile " + configurationFile, e);
82-
}
83-
}
84-
8556
File[] files = getFiles();
8657

8758
int failed = 0;
8859

8960
if(files.length>0) {
9061

91-
DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(settings);
92-
for (int i = 0; i < files.length; i++) {
93-
File file = files[i];
94-
try {
95-
boolean ok = formatter.formatFile( file );
96-
if(!ok) {
97-
failed++;
98-
getProject().log(this, "Formatting failed - skipping " + file, Project.MSG_WARN);
99-
} else {
100-
getProject().log(this, "Formatted " + file, Project.MSG_VERBOSE);
101-
}
102-
} catch(RuntimeException ee) {
103-
failed++;
104-
if(failOnError) {
105-
throw new BuildException("Java formatting failed on " + file, ee);
106-
} else {
107-
getProject().log(this, "Java formatting failed on " + file + ", " + ee.getLocalizedMessage(), Project.MSG_ERR);
108-
}
109-
}
110-
}
62+
DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy();
63+
for (File file : files) {
64+
try {
65+
boolean ok = formatter.formatFile(file);
66+
if (!ok) {
67+
failed++;
68+
getProject().log(this, "Formatting failed - skipping " + file, Project.MSG_WARN);
69+
} else {
70+
getProject().log(this, "Formatted " + file, Project.MSG_VERBOSE);
71+
}
72+
} catch (RuntimeException ee) {
73+
failed++;
74+
if (failOnError) {
75+
throw new BuildException("Java formatting failed on " + file, ee);
76+
} else {
77+
getProject().log(this, "Java formatting failed on " + file + ", " + ee.getLocalizedMessage(), Project.MSG_ERR);
78+
}
79+
}
80+
}
11181
}
11282

11383
getProject().log( this, "Java formatting of " + files.length + " files completed. Skipped " + failed + " file(s).", Project.MSG_INFO );
@@ -117,23 +87,22 @@ public void execute() throws BuildException {
11787
private File[] getFiles() {
11888

11989
List<File> files = new LinkedList<File>();
120-
for ( Iterator<FileSet> i = fileSets.iterator(); i.hasNext(); ) {
90+
for (FileSet fs : fileSets) {
12191

122-
FileSet fs = i.next();
123-
DirectoryScanner ds = fs.getDirectoryScanner( getProject() );
92+
DirectoryScanner ds = fs.getDirectoryScanner(getProject());
12493

125-
String[] dsFiles = ds.getIncludedFiles();
126-
for (int j = 0; j < dsFiles.length; j++) {
127-
File f = new File(dsFiles[j]);
128-
if ( !f.isFile() ) {
129-
f = new File( ds.getBasedir(), dsFiles[j] );
130-
}
94+
String[] dsFiles = ds.getIncludedFiles();
95+
for (String dsFile : dsFiles) {
96+
File f = new File(dsFile);
97+
if (!f.isFile()) {
98+
f = new File(ds.getBasedir(), dsFile);
99+
}
131100

132-
files.add( f );
133-
}
134-
}
101+
files.add(f);
102+
}
103+
}
135104

136-
return (File[]) files.toArray(new File[files.size()]);
105+
return (File[]) files.toArray(new File[0]);
137106
}
138107

139108
}

‎orm/src/main/java/org/hibernate/tool/api/java/DefaultJavaPrettyPrinterStrategy.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
import com.google.googlejavaformat.java.FormatterException;
2727

2828
public class DefaultJavaPrettyPrinterStrategy {
29-
30-
public DefaultJavaPrettyPrinterStrategy(Map<Object, Object> settings) {}
3129

3230
public boolean formatFile(File file) {
3331
try {
@@ -40,5 +38,5 @@ public boolean formatFile(File file) {
4038
throw new RuntimeException(e);
4139
}
4240
}
43-
44-
}
41+
42+
}

‎test/nodb/src/test/java/org/hibernate/tool/ant/JavaFormatter/TestCase.java‎

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void testJavaFormatFile() {
7171
.findFirstString("public", simpleOne)
7272
.contains("SimpleOne"));
7373

74-
DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(null);
74+
DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy();
7575
formatter.formatFile( simpleOne );
7676

7777
assertTrue(FileUtil
@@ -108,7 +108,7 @@ public void testJavaJdk5FormatFile() {
108108

109109
assertFalse(log.contains("Exception"));
110110

111-
DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(null);
111+
DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy();
112112
assertTrue(
113113
formatter.formatFile(simple5One),
114114
"formatting should pass when using default settings");
@@ -170,53 +170,31 @@ public void testConfig() {
170170

171171
String log = AntUtil.getLog(project);
172172
assertFalse(log.contains("Exception"));
173-
173+
174174
assertTrue(simpleOne.exists());
175175
assertTrue(simple5One.exists());
176-
176+
177177
assertFalse(FileUtil
178178
.findFirstString("public", simpleOne)
179179
.contains("SimpleOne"));
180180
assertFalse(FileUtil
181181
.findFirstString("public", simple5One)
182182
.contains("Simple5One"));
183183

184-
project.executeTarget("configtest");
184+
project.executeTarget("formatfiles");
185185
log = AntUtil.getLog(project);
186186
assertFalse(log.contains("Exception"));
187-
187+
188188
assertTrue(FileUtil
189189
.findFirstString("public", simpleOne)
190190
.contains("SimpleOne"));
191191
assertTrue(FileUtil
192192
.findFirstString("public", simple5One)
193193
.contains("Simple5One"));
194-
194+
195195
assertTrue(simpleOne.delete());
196196
assertTrue(simple5One.delete());
197-
198-
project.executeTarget("copyfiles");
199-
log = AntUtil.getLog(project);
200-
assertFalse(log.contains("Exception"));
201197

202-
assertFalse(FileUtil
203-
.findFirstString("public", simpleOne)
204-
.contains("SimpleOne"));
205-
assertFalse(FileUtil
206-
.findFirstString("public", simple5One)
207-
.contains("Simple5One"));
208-
209-
project.executeTarget("noconfigtest");
210-
log = AntUtil.getLog(project);
211-
assertFalse(log.contains("Exception"));
212-
213-
assertTrue(FileUtil
214-
.findFirstString("public", simpleOne)
215-
.contains("SimpleOne"));
216-
assertTrue(FileUtil
217-
.findFirstString("public", simple5One)
218-
.contains("Simple5One"));
219-
220198
}
221199

222200
}

‎test/nodb/src/test/resources/org/hibernate/tool/ant/JavaFormatter/build.xml‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -55,36 +55,5 @@
5555
</javaformatter>
5656

5757
</target>
58-
59-
60-
<target name="configtest">
61-
62-
<taskdef
63-
name="javaformatter"
64-
classname="org.hibernate.tool.ant.JavaFormatterTask"/>
65-
66-
<javaformatter configurationfile="${resourcesDir}/emptyconfig.properties">
67-
<fileset dir="${destinationDir}">
68-
<include name="formatting/**/*"/>
69-
</fileset>
70-
</javaformatter>
71-
72-
</target>
73-
74-
75-
<target name="noconfigtest">
76-
77-
<taskdef
78-
name="javaformatter"
79-
classname="org.hibernate.tool.ant.JavaFormatterTask"/>
80-
81-
<javaformatter>
82-
<fileset dir="${destinationDir}">
83-
<include name="formatting/**/*"/>
84-
</fileset>
85-
</javaformatter>
86-
87-
</target>
88-
8958

9059
</project>

0 commit comments

Comments
 (0)