Skip to content

Commit

Permalink
Fix deprecated uses of newInstance() (#1887)
Browse files Browse the repository at this point in the history 
* Fix deprecated uses of newInstance()
* Fix suppression of exception which could cause test to erroneously pass.
  • Loading branch information
@lbergelson
lbergelson authored Jun 2, 2023
1 parent 17538c1 commit 4527d6d
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 24 deletions.
17 changes: 8 additions & 9 deletions src/main/java/picard/cmdline/PicardCommandLine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
import org.broadinstitute.barclay.argparser.ExperimentalFeature;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -150,12 +151,11 @@ else if (!clProperties.omitFromCommandLine()) { /** We should check for missing
printCommandList(classes);
} else {
if (simpleNameToClass.containsKey(args[0])) {
final Class clazz = simpleNameToClass.get(args[0]);
final Class<?> clazz = simpleNameToClass.get(args[0]);
try {
return (CommandLineProgram)clazz.newInstance();
} catch (final InstantiationException e) {
throw new RuntimeException(e);
} catch (final IllegalAccessException e) {
return (CommandLineProgram)clazz.getDeclaredConstructor().newInstance();
} catch (final InstantiationException | IllegalAccessException
| InvocationTargetException | NoSuchMethodException e) {
throw new RuntimeException(e);
}
}
Expand Down Expand Up @@ -229,10 +229,9 @@ private static void printUsage(final Set<Class<?>> classes, final String command
CommandLineProgramGroup programGroup = programGroupClassToProgramGroupInstance.get(property.programGroup());
if (null == programGroup) {
try {
programGroup = property.programGroup().newInstance();
} catch (final InstantiationException e) {
throw new RuntimeException(e);
} catch (final IllegalAccessException e) {
programGroup = property.programGroup().getDeclaredConstructor().newInstance();
} catch (final InstantiationException | IllegalAccessException
| NoSuchMethodException | InvocationTargetException e) {
throw new RuntimeException(e);
}
programGroupClassToProgramGroupInstance.put(property.programGroup(), programGroup);
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/picard/sam/MergeBamAlignment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,22 +310,22 @@ enum PrimaryAlignmentStrategy implements CommandLineParser.ClpEnum{
"the alignment pair with the largest insert size. If all alignments would be chimeric, it picks the " +
"alignments for each end with the best MAPQ. ");

private final Class<PrimaryAlignmentSelectionStrategy> clazz;
private final Class<? extends PrimaryAlignmentSelectionStrategy> clazz;

private final String description;

public String getHelpDoc() {
return description;
}

PrimaryAlignmentStrategy(final Class<?> clazz, final String description) {
this.clazz = (Class<PrimaryAlignmentSelectionStrategy>) clazz;
PrimaryAlignmentStrategy(final Class<? extends PrimaryAlignmentSelectionStrategy> clazz, final String description) {
this.clazz = clazz;
this.description = description;
}

PrimaryAlignmentSelectionStrategy newInstance() {
try {
return clazz.newInstance();
return clazz.getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new PicardException("Trouble instantiating " + clazz.getName(), e);
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/picard/TestDataProviders.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ public Iterator<Object[]> testAllDataProvidersData() throws Exception {
// https://github.com/cbeust/testng/blob/master/src/test/java/test/inject/NoInjectionTest.java
@Test(dataProvider = "DataprovidersThatDontTestThemselves")
public void testDataProviderswithDP(@NoInjection final Method method, final Class clazz) throws
IllegalAccessException, InstantiationException {
IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException {

Object instance = clazz.newInstance();
Object instance = clazz.getDeclaredConstructor().newInstance();

Set<Method> methodSet = new HashSet<>();
methodSet.addAll(Arrays.asList(clazz.getDeclaredMethods()));
Expand Down
6 changes: 4 additions & 2 deletions src/test/java/picard/cmdline/PicardCommandLineTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import java.lang.reflect.InvocationTargetException;
import java.util.*;

public class PicardCommandLineTest {
Expand Down Expand Up @@ -50,13 +52,13 @@ public void testLaunchAllCommandLineProgramsWithBarclayParser() {
// Check for missing annotations
Assert.assertNotNull(clProperties);
try {
final Object commandLineProgram = clazz.newInstance();
final Object commandLineProgram = clazz.getDeclaredConstructor().newInstance();
try {
new CommandLineArgumentParser(commandLineProgram);
} catch (CommandLineException.CommandLineParserInternalException e) {
throw new RuntimeException("Barclay command line parser internal exception parsing class: " + clazz.getName(), e);
}
} catch (IllegalAccessException | InstantiationException e) {
} catch (IllegalAccessException | InstantiationException | NoSuchMethodException | InvocationTargetException e) {
throw new RuntimeException("Failure instantiating command line program: " + clazz.getName(), e);
}
});
Expand Down
21 changes: 14 additions & 7 deletions src/test/java/picard/sam/CramCompatibilityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
import htsjdk.samtools.SamStreams;
import htsjdk.samtools.cram.CRAMException;
import htsjdk.samtools.util.IOUtil;
import htsjdk.samtools.util.RuntimeIOException;
import org.testng.Assert;
import org.testng.annotations.AfterTest;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import picard.cmdline.CommandLineProgram;

import java.io.*;
import java.lang.reflect.InvocationTargetException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand Down Expand Up @@ -108,7 +110,7 @@ public Object[][] getArgsForCRAMWithReference() {
public void testShouldWriteCRAMWhenCRAMWithReference(String program,
String parameters,
String cramFile,
String reference) throws IOException, IllegalAccessException, InstantiationException, ClassNotFoundException {
String reference) throws IOException {
if (!program.equals("picard.sam.SplitSamByLibrary")) {
final File outputFile = createTempCram(program);
launchProgram(program, cramFile, outputFile.getAbsolutePath(), parameters, reference);
Expand Down Expand Up @@ -160,7 +162,7 @@ public Object[][] getArgsForCRAMWithoutReferenceToFail() {
@Test(dataProvider = "programArgsForCRAMWithoutReferenceToFail", expectedExceptions = {CRAMException.class, IllegalArgumentException.class})
public void testShouldFailWhenCRAMWithoutReference(String program,
String parameters,
String cramFile) throws IOException, IllegalAccessException, InstantiationException, ClassNotFoundException {
String cramFile) throws IOException {
if (!program.equals("picard.sam.SplitSamByLibrary")) {
final File outputFile = createTempCram(program);
launchProgram(program, cramFile, outputFile.getAbsolutePath(), parameters, null);
Expand Down Expand Up @@ -210,7 +212,7 @@ public Object[][] getArgsWithUnmappedCRAM() {
@Test(dataProvider = "programArgsWithUnmappedCRAM")
public void testShouldWriteCRAMWhenUnmappedCRAMWithoutReference(String program,
String parameters,
String cramFile) throws IOException, IllegalAccessException, InstantiationException, ClassNotFoundException {
String cramFile) throws IOException {
if (!program.equals("picard.sam.SplitSamByLibrary")) {
final File outputFile = createTempCram(program);
launchProgram(program, cramFile, outputFile.getAbsolutePath(), parameters, null);
Expand All @@ -222,7 +224,7 @@ public void testShouldWriteCRAMWhenUnmappedCRAMWithoutReference(String program,
}
}

private File createTempCram(String name) throws IOException {
private File createTempCram(String name) {
return createTempFile(name, ".cram");
}

Expand All @@ -242,7 +244,7 @@ private void launchProgram(String programClassname,
String input,
String output,
String exParams,
String reference) throws ClassNotFoundException, IllegalAccessException, InstantiationException {
String reference) {
final Collection<String> args = new ArrayList<>();

if (input != null) {
Expand All @@ -258,7 +260,12 @@ private void launchProgram(String programClassname,
args.add("REFERENCE_SEQUENCE=" + new File(reference).getAbsolutePath());
}

final CommandLineProgram program = (CommandLineProgram) Class.forName(programClassname).newInstance();
final CommandLineProgram program;
try {
program = (CommandLineProgram) Class.forName(programClassname).getDeclaredConstructor().newInstance();
} catch (ReflectiveOperationException e){
throw new RuntimeException(e);
}
program.instanceMain(args.toArray(new String[0]));
}

Expand All @@ -267,7 +274,7 @@ static void assertCRAM(final File outputFile) {
try (InputStream in = new FileInputStream(outputFile)) {
Assert.assertTrue(SamStreams.isCRAMFile(new BufferedInputStream(in)), "File " + outputFile.getAbsolutePath() + " is not a CRAM.");
} catch (IOException e) {
e.printStackTrace();
throw new RuntimeIOException(e);
}
}

Expand Down

0 comments on commit 4527d6d

Please sign in to comment.