Skip to content

Commit 9a380f6

Browse files
eilensmmaei
eilensm
and
maei
authored
Issue #111 - Refactor and add unit tests for support for multiple --add-exp… (#113)
* #111 - Refactor and add unit tests for support for multiple --add-exports custom compiler arguments * #111 - Minor formatting and variable refactoring Co-authored-by: maei <marc.eilens@parcit.de>
1 parent 0a0aab0 commit 9a380f6

File tree

3 files changed

+195
-78
lines changed

3 files changed

+195
-78
lines changed

‎plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java

Lines changed: 83 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import java.util.Iterator;
5353
import java.util.List;
5454
import java.util.Locale;
55-
import java.util.Map;
5655
import java.util.Map.Entry;
5756
import java.util.ServiceLoader;
5857
import java.util.Set;
@@ -152,76 +151,7 @@ public CompilerResult performCompile( CompilerConfiguration config )
152151

153152
// Set Eclipse-specific options
154153
// compiler-specific extra options override anything else in the config object...
155-
Map<String, String> extras = config.getCustomCompilerArgumentsAsMap();
156-
if ( extras.containsKey( "errorsAsWarnings" ) )
157-
{
158-
extras.remove( "errorsAsWarnings" );
159-
this.errorsAsWarnings = true;
160-
}
161-
else if ( extras.containsKey( "-errorsAsWarnings" ) )
162-
{
163-
extras.remove( "-errorsAsWarnings" );
164-
this.errorsAsWarnings = true;
165-
}
166-
167-
//-- check for existence of the properties file manually
168-
String props = extras.get( "-properties" );
169-
if ( null != props )
170-
{
171-
File propFile = new File( props );
172-
if ( !propFile.exists() || !propFile.isFile() )
173-
{
174-
throw new IllegalArgumentException(
175-
"Properties file specified by -properties " + propFile + " does not exist" );
176-
}
177-
}
178-
179-
for ( Entry<String, String> entry : extras.entrySet() )
180-
{
181-
/*
182-
* The compiler mojo makes quite a mess of passing arguments, depending on exactly WHICH
183-
* way is used to pass them. The method method using <compilerArguments> uses the tag names
184-
* of its contents to denote option names, and so the compiler mojo happily adds a '-' to
185-
* all of the names there and adds them to the "custom compiler arguments" map as a
186-
* name, value pair where the name always contains a single '-'. The Eclipse compiler (and
187-
* javac too, btw) has options with two dashes (like --add-modules for java 9). These cannot
188-
* be passed using a <compilerArguments> tag.
189-
*
190-
* The other method is to use <compilerArgs>, where each SINGLE argument needs to be passed
191-
* using an <arg>xxxx</arg> tag. In there the xxx is not manipulated by the compiler mojo, so
192-
* if it starts with a dash or more dashes these are perfectly preserved. But of course these
193-
* single <arg> entries are not a pair. So the compiler mojo adds them as pairs of (xxxx, null).
194-
*
195-
* We use that knowledge here: if a pair has a null value then do not mess up the key but
196-
* render it as a single value. This should ensure that something like:
197-
* <compilerArgs>
198-
* <arg>--add-modules</arg>
199-
* <arg>java.se.ee</arg>
200-
* </compilerArgs>
201-
*
202-
* is actually added to the command like as such.
203-
*
204-
* (btw: the above example will still give an error when using ecj <= 4.8M6:
205-
* invalid module name: java.se.ee
206-
* but that seems to be a bug in ecj).
207-
*/
208-
String opt = entry.getKey();
209-
String optionValue = entry.getValue();
210-
if ( null == optionValue )
211-
{
212-
//-- We have an option from compilerArgs: use the key as-is as a single option value
213-
args.add( opt );
214-
}
215-
else
216-
{
217-
if ( !opt.startsWith( "-" ) )
218-
{
219-
opt = "-" + opt;
220-
}
221-
args.add( opt );
222-
args.add( optionValue );
223-
}
224-
}
154+
this.errorsAsWarnings = processCustomArguments(config, args);
225155

226156
// Output path
227157
args.add( "-d" );
@@ -275,13 +205,6 @@ else if ( extras.containsKey( "-errorsAsWarnings" ) )
275205
}
276206
}
277207

278-
//-- Write .class files even when error occur, but make sure methods with compile errors do abort when called
279-
if ( extras.containsKey( "-proceedOnError" ) )
280-
{
281-
// Generate a class file even with errors, but make methods with errors fail when called
282-
args.add( "-proceedOnError:Fatal" );
283-
}
284-
285208
//-- classpath
286209
List<String> classpathEntries = new ArrayList<>( config.getClasspathEntries() );
287210
classpathEntries.add( config.getOutputLocation() );
@@ -528,6 +451,88 @@ public void worked( int i, int i1 )
528451
}
529452
}
530453

454+
static boolean processCustomArguments( CompilerConfiguration config, List<String> args )
455+
{
456+
boolean result = false;
457+
458+
for ( Entry<String, String> entry : config.getCustomCompilerArgumentsEntries() )
459+
{
460+
String opt = entry.getKey();
461+
String optionValue = entry.getValue();
462+
463+
// handle errorsAsWarnings options
464+
if ( opt.equals("errorsAsWarnings") || opt.equals("-errorsAsWarnings") )
465+
{
466+
result = true;
467+
continue;
468+
}
469+
470+
if ( opt.equals( "-properties" ) )
471+
{
472+
if ( null != optionValue )
473+
{
474+
File propFile = new File( optionValue );
475+
if ( !propFile.exists() || !propFile.isFile() )
476+
{
477+
throw new IllegalArgumentException(
478+
"Properties file specified by -properties " + propFile + " does not exist" );
479+
}
480+
}
481+
}
482+
483+
//-- Write .class files even when error occur, but make sure methods with compile errors do abort when called
484+
if ( opt.equals( "-proceedOnError" ) )
485+
{
486+
// Generate a class file even with errors, but make methods with errors fail when called
487+
args.add( "-proceedOnError:Fatal" );
488+
continue;
489+
}
490+
491+
/*
492+
* The compiler mojo makes quite a mess of passing arguments, depending on exactly WHICH
493+
* way is used to pass them. The method method using <compilerArguments> uses the tag names
494+
* of its contents to denote option names, and so the compiler mojo happily adds a '-' to
495+
* all of the names there and adds them to the "custom compiler arguments" map as a
496+
* name, value pair where the name always contains a single '-'. The Eclipse compiler (and
497+
* javac too, btw) has options with two dashes (like --add-modules for java 9). These cannot
498+
* be passed using a <compilerArguments> tag.
499+
*
500+
* The other method is to use <compilerArgs>, where each SINGLE argument needs to be passed
501+
* using an <arg>xxxx</arg> tag. In there the xxx is not manipulated by the compiler mojo, so
502+
* if it starts with a dash or more dashes these are perfectly preserved. But of course these
503+
* single <arg> entries are not a pair. So the compiler mojo adds them as pairs of (xxxx, null).
504+
*
505+
* We use that knowledge here: if a pair has a null value then do not mess up the key but
506+
* render it as a single value. This should ensure that something like:
507+
* <compilerArgs>
508+
* <arg>--add-modules</arg>
509+
* <arg>java.se.ee</arg>
510+
* </compilerArgs>
511+
*
512+
* is actually added to the command like as such.
513+
*
514+
* (btw: the above example will still give an error when using ecj <= 4.8M6:
515+
* invalid module name: java.se.ee
516+
* but that seems to be a bug in ecj).
517+
*/
518+
if ( null == optionValue )
519+
{
520+
//-- We have an option from compilerArgs: use the key as-is as a single option value
521+
args.add( opt );
522+
}
523+
else
524+
{
525+
if ( !opt.startsWith( "-" ) )
526+
{
527+
opt = "-" + opt;
528+
}
529+
args.add( opt );
530+
args.add( optionValue );
531+
}
532+
}
533+
return result;
534+
}
535+
531536
private static boolean haveSourceOrReleaseArgument( List<String> args )
532537
{
533538
Iterator<String> allArgs = args.iterator();

‎plexus-compilers/plexus-compiler-eclipse/src/test/java/org/codehaus/plexus/compiler/eclipse/EclipseCompilerConfigurationTest.java

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
package org.codehaus.plexus.compiler.eclipse;
2+
3+
/**
4+
* The MIT License
5+
*
6+
* Copyright (c) 2005, The Codehaus
7+
*
8+
* Permission is hereby granted, free of charge, to any person obtaining a copy of
9+
* this software and associated documentation files (the "Software"), to deal in
10+
* the Software without restriction, including without limitation the rights to
11+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
12+
* of the Software, and to permit persons to whom the Software is furnished to do
13+
* so, subject to the following conditions:
14+
*
15+
* The above copyright notice and this permission notice shall be included in all
16+
* copies or substantial portions of the Software.
17+
*
18+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
19+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
20+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
21+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
22+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
23+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
24+
* SOFTWARE.
25+
*/
26+
27+
import junit.framework.TestCase;
28+
import org.codehaus.plexus.compiler.CompilerConfiguration;
29+
30+
import java.io.File;
31+
import java.util.ArrayList;
32+
import java.util.Collections;
33+
import java.util.List;
34+
35+
public class EclipseCompilerConfigurationTest
36+
extends TestCase
37+
{
38+
private static final String PROPERTIES_FILE_NAME =
39+
"src/test/resources".replace( "/", File.separator ) + File.separator
40+
+ EclipseCompilerConfigurationTest.class.getName().replace( ".", File.separator )
41+
+ "-test.properties";
42+
43+
private CompilerConfiguration configuration;
44+
45+
@Override
46+
protected void setUp()
47+
{
48+
configuration = new CompilerConfiguration();
49+
}
50+
51+
public void testProcessCustomArgumentsWithMultipleAddExports()
52+
{
53+
configuration.addCompilerCustomArgument( "--add-exports", "FROM-MOD/package1=OTHER-MOD" );
54+
configuration.addCompilerCustomArgument( "--add-exports", "FROM-MOD/package2=OTHER-MOD" );
55+
List<String> args = new ArrayList<>();
56+
EclipseJavaCompiler.processCustomArguments( configuration, args );
57+
assertEquals( 4, args.size() );
58+
assertEquals( "--add-exports", args.get( 0 ) );
59+
assertEquals( "FROM-MOD/package1=OTHER-MOD", args.get( 1 ) );
60+
assertEquals( "--add-exports", args.get( 2 ) );
61+
assertEquals( "FROM-MOD/package2=OTHER-MOD", args.get( 3 ) );
62+
}
63+
64+
public void testProcessCustomArgumentsWithProceedOnError()
65+
{
66+
configuration.addCompilerCustomArgument( "-proceedOnError", null );
67+
List<String> args = new ArrayList<>();
68+
EclipseJavaCompiler.processCustomArguments( configuration, args );
69+
assertEquals( 1, args.size() );
70+
assertEquals( "-proceedOnError:Fatal", args.get( 0 ) );
71+
}
72+
73+
public void testProcessCustomArgumentsWithErrorsAsWarnings()
74+
{
75+
configuration.addCompilerCustomArgument( "errorsAsWarnings", null );
76+
List<String> args = new ArrayList<>();
77+
final boolean errorsAsWarnings = EclipseJavaCompiler.processCustomArguments( configuration, args );
78+
assertTrue( errorsAsWarnings );
79+
}
80+
81+
public void testProcessCustomArgumentsWithErrorsAsWarningsAndMinus()
82+
{
83+
configuration.addCompilerCustomArgument( "-errorsAsWarnings", null );
84+
List<String> args = new ArrayList<>();
85+
final boolean errorsAsWarnings = EclipseJavaCompiler.processCustomArguments( configuration, args );
86+
assertTrue( errorsAsWarnings );
87+
}
88+
89+
public void testProcessCustomArgumentsWithPropertiesAndNonExistingFile()
90+
{
91+
configuration.addCompilerCustomArgument( "-properties", "fooBar.txt" );
92+
try
93+
{
94+
EclipseJavaCompiler.processCustomArguments( configuration, Collections.<String>emptyList() );
95+
fail( "IllegalArgumentException expected" );
96+
} catch ( IllegalArgumentException expected )
97+
{
98+
assertEquals( "Properties file specified by -properties fooBar.txt does not exist", expected.getMessage() );
99+
}
100+
}
101+
102+
public void testProcessCustomArgumentsWithPropertiesAndValidFile()
103+
{
104+
configuration.addCompilerCustomArgument( "-properties", PROPERTIES_FILE_NAME );
105+
List<String> args = new ArrayList<>();
106+
EclipseJavaCompiler.processCustomArguments( configuration, args );
107+
assertEquals( 2, args.size() );
108+
assertEquals( "-properties", args.get( 0 ) );
109+
assertEquals( PROPERTIES_FILE_NAME, args.get( 1 ) );
110+
}
111+
}

‎plexus-compilers/plexus-compiler-eclipse/src/test/resources/org/codehaus/plexus/compiler/eclipse/EclipseCompilerConfigurationTest-test.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
foo=bar

0 commit comments

Comments
 (0)