Skip to content

Issue #111 - Refactor and add unit tests for support for multiple --add-exp… #113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 30, 2020
Merged

Issue #111 - Refactor and add unit tests for support for multiple --add-exp… #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b47d120
#111 - Refactor and add unit tests for support for multiple --add-exp…
Dec 6, 2020
46e94bf
#111 - Minor formatting and variable refactoring
Dec 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 83 additions & 78 deletions ...piler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.ServiceLoader;
import java.util.Set;
Expand Down Expand Up @@ -152,76 +151,7 @@ public CompilerResult performCompile( CompilerConfiguration config )

// Set Eclipse-specific options
// compiler-specific extra options override anything else in the config object...
Map<String, String> extras = config.getCustomCompilerArgumentsAsMap();
if ( extras.containsKey( "errorsAsWarnings" ) )
{
extras.remove( "errorsAsWarnings" );
this.errorsAsWarnings = true;
}
else if ( extras.containsKey( "-errorsAsWarnings" ) )
{
extras.remove( "-errorsAsWarnings" );
this.errorsAsWarnings = true;
}

//-- check for existence of the properties file manually
String props = extras.get( "-properties" );
if ( null != props )
{
File propFile = new File( props );
if ( !propFile.exists() || !propFile.isFile() )
{
throw new IllegalArgumentException(
"Properties file specified by -properties " + propFile + " does not exist" );
}
}

for ( Entry<String, String> entry : extras.entrySet() )
{
/*
* The compiler mojo makes quite a mess of passing arguments, depending on exactly WHICH
* way is used to pass them. The method method using <compilerArguments> uses the tag names
* of its contents to denote option names, and so the compiler mojo happily adds a '-' to
* all of the names there and adds them to the "custom compiler arguments" map as a
* name, value pair where the name always contains a single '-'. The Eclipse compiler (and
* javac too, btw) has options with two dashes (like --add-modules for java 9). These cannot
* be passed using a <compilerArguments> tag.
*
* The other method is to use <compilerArgs>, where each SINGLE argument needs to be passed
* using an <arg>xxxx</arg> tag. In there the xxx is not manipulated by the compiler mojo, so
* if it starts with a dash or more dashes these are perfectly preserved. But of course these
* single <arg> entries are not a pair. So the compiler mojo adds them as pairs of (xxxx, null).
*
* We use that knowledge here: if a pair has a null value then do not mess up the key but
* render it as a single value. This should ensure that something like:
* <compilerArgs>
* <arg>--add-modules</arg>
* <arg>java.se.ee</arg>
* </compilerArgs>
*
* is actually added to the command like as such.
*
* (btw: the above example will still give an error when using ecj <= 4.8M6:
* invalid module name: java.se.ee
* but that seems to be a bug in ecj).
*/
String opt = entry.getKey();
String optionValue = entry.getValue();
if ( null == optionValue )
{
//-- We have an option from compilerArgs: use the key as-is as a single option value
args.add( opt );
}
else
{
if ( !opt.startsWith( "-" ) )
{
opt = "-" + opt;
}
args.add( opt );
args.add( optionValue );
}
}
this.errorsAsWarnings = processCustomArguments(config, args);

// Output path
args.add( "-d" );
Expand Down Expand Up @@ -275,13 +205,6 @@ else if ( extras.containsKey( "-errorsAsWarnings" ) )
}
}

//-- Write .class files even when error occur, but make sure methods with compile errors do abort when called
if ( extras.containsKey( "-proceedOnError" ) )
{
// Generate a class file even with errors, but make methods with errors fail when called
args.add( "-proceedOnError:Fatal" );
}

//-- classpath
List<String> classpathEntries = new ArrayList<>( config.getClasspathEntries() );
classpathEntries.add( config.getOutputLocation() );
Expand Down Expand Up @@ -528,6 +451,88 @@ public void worked( int i, int i1 )
}
}

static boolean processCustomArguments( CompilerConfiguration config, List<String> args )
{
boolean result = false;

for ( Entry<String, String> entry : config.getCustomCompilerArgumentsEntries() )
{
String opt = entry.getKey();
String optionValue = entry.getValue();

// handle errorsAsWarnings options
if ( opt.equals("errorsAsWarnings") || opt.equals("-errorsAsWarnings") )
{
result = true;
continue;
}

if ( opt.equals( "-properties" ) )
{
if ( null != optionValue )
{
File propFile = new File( optionValue );
if ( !propFile.exists() || !propFile.isFile() )
{
throw new IllegalArgumentException(
"Properties file specified by -properties " + propFile + " does not exist" );
}
}
}

//-- Write .class files even when error occur, but make sure methods with compile errors do abort when called
if ( opt.equals( "-proceedOnError" ) )
{
// Generate a class file even with errors, but make methods with errors fail when called
args.add( "-proceedOnError:Fatal" );
continue;
}

/*
* The compiler mojo makes quite a mess of passing arguments, depending on exactly WHICH
* way is used to pass them. The method method using <compilerArguments> uses the tag names
* of its contents to denote option names, and so the compiler mojo happily adds a '-' to
* all of the names there and adds them to the "custom compiler arguments" map as a
* name, value pair where the name always contains a single '-'. The Eclipse compiler (and
* javac too, btw) has options with two dashes (like --add-modules for java 9). These cannot
* be passed using a <compilerArguments> tag.
*
* The other method is to use <compilerArgs>, where each SINGLE argument needs to be passed
* using an <arg>xxxx</arg> tag. In there the xxx is not manipulated by the compiler mojo, so
* if it starts with a dash or more dashes these are perfectly preserved. But of course these
* single <arg> entries are not a pair. So the compiler mojo adds them as pairs of (xxxx, null).
*
* We use that knowledge here: if a pair has a null value then do not mess up the key but
* render it as a single value. This should ensure that something like:
* <compilerArgs>
* <arg>--add-modules</arg>
* <arg>java.se.ee</arg>
* </compilerArgs>
*
* is actually added to the command like as such.
*
* (btw: the above example will still give an error when using ecj <= 4.8M6:
* invalid module name: java.se.ee
* but that seems to be a bug in ecj).
*/
if ( null == optionValue )
{
//-- We have an option from compilerArgs: use the key as-is as a single option value
args.add( opt );
}
else
{
if ( !opt.startsWith( "-" ) )
{
opt = "-" + opt;
}
args.add( opt );
args.add( optionValue );
}
}
return result;
}

private static boolean haveSourceOrReleaseArgument( List<String> args )
{
Iterator<String> allArgs = args.iterator();
Expand Down
111 changes: 111 additions & 0 deletions .../src/test/java/org/codehaus/plexus/compiler/eclipse/EclipseCompilerConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package org.codehaus.plexus.compiler.eclipse;

/**
* The MIT License
*
* Copyright (c) 2005, The Codehaus
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
* of the Software, and to permit persons to whom the Software is furnished to do
* so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/

import junit.framework.TestCase;
import org.codehaus.plexus.compiler.CompilerConfiguration;

import java.io.File;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class EclipseCompilerConfigurationTest
extends TestCase
{
private static final String PROPERTIES_FILE_NAME =
"src/test/resources".replace( "/", File.separator ) + File.separator
+ EclipseCompilerConfigurationTest.class.getName().replace( ".", File.separator )
+ "-test.properties";

private CompilerConfiguration configuration;

@Override
protected void setUp()
{
configuration = new CompilerConfiguration();
}

public void testProcessCustomArgumentsWithMultipleAddExports()
{
configuration.addCompilerCustomArgument( "--add-exports", "FROM-MOD/package1=OTHER-MOD" );
configuration.addCompilerCustomArgument( "--add-exports", "FROM-MOD/package2=OTHER-MOD" );
List<String> args = new ArrayList<>();
EclipseJavaCompiler.processCustomArguments( configuration, args );
assertEquals( 4, args.size() );
assertEquals( "--add-exports", args.get( 0 ) );
assertEquals( "FROM-MOD/package1=OTHER-MOD", args.get( 1 ) );
assertEquals( "--add-exports", args.get( 2 ) );
assertEquals( "FROM-MOD/package2=OTHER-MOD", args.get( 3 ) );
}

public void testProcessCustomArgumentsWithProceedOnError()
{
configuration.addCompilerCustomArgument( "-proceedOnError", null );
List<String> args = new ArrayList<>();
EclipseJavaCompiler.processCustomArguments( configuration, args );
assertEquals( 1, args.size() );
assertEquals( "-proceedOnError:Fatal", args.get( 0 ) );
}

public void testProcessCustomArgumentsWithErrorsAsWarnings()
{
configuration.addCompilerCustomArgument( "errorsAsWarnings", null );
List<String> args = new ArrayList<>();
final boolean errorsAsWarnings = EclipseJavaCompiler.processCustomArguments( configuration, args );
assertTrue( errorsAsWarnings );
}

public void testProcessCustomArgumentsWithErrorsAsWarningsAndMinus()
{
configuration.addCompilerCustomArgument( "-errorsAsWarnings", null );
List<String> args = new ArrayList<>();
final boolean errorsAsWarnings = EclipseJavaCompiler.processCustomArguments( configuration, args );
assertTrue( errorsAsWarnings );
}

public void testProcessCustomArgumentsWithPropertiesAndNonExistingFile()
{
configuration.addCompilerCustomArgument( "-properties", "fooBar.txt" );
try
{
EclipseJavaCompiler.processCustomArguments( configuration, Collections.<String>emptyList() );
fail( "IllegalArgumentException expected" );
} catch ( IllegalArgumentException expected )
{
assertEquals( "Properties file specified by -properties fooBar.txt does not exist", expected.getMessage() );
}
}

public void testProcessCustomArgumentsWithPropertiesAndValidFile()
{
configuration.addCompilerCustomArgument( "-properties", PROPERTIES_FILE_NAME );
List<String> args = new ArrayList<>();
EclipseJavaCompiler.processCustomArguments( configuration, args );
assertEquals( 2, args.size() );
assertEquals( "-properties", args.get( 0 ) );
assertEquals( PROPERTIES_FILE_NAME, args.get( 1 ) );
}
}
1 change: 1 addition & 0 deletions ...ces/org/codehaus/plexus/compiler/eclipse/EclipseCompilerConfigurationTest-test.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo=bar