From a551cdd5041e9ad26f4924b88aa6cb83205c5a43 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 3 Aug 2018 12:48:04 +0200 Subject: [PATCH] fix(patch): Improve patches generation (#13) --- .gitignore | 3 +- .../npefix/resi/NPEFixTemplateEvaluation.java | 37 +++++++------- .../spirals/npefix/main/ExecutionClient.java | 8 +-- .../main/all/DefaultRepairStrategy.java | 8 +-- .../spirals/npefix/main/all/Launcher.java | 2 +- .../main/all/TryCatchRepairStrategy.java | 3 +- .../spirals/npefix/main/patch/SortPatch.java | 2 +- .../patch/generator/PatchesGenerator.java | 32 ++++++++---- .../spirals/npefix/resi/context/Lapse.java | 6 ++- .../npefix/patch/PatchesGeneratorTest.java | 50 ++++++++++--------- .../transformer/TryCatchRepairModelTest.java | 3 +- 11 files changed, 90 insertions(+), 64 deletions(-) diff --git a/.gitignore b/.gitignore index e4ce971..9e377e9 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,5 @@ buildNumber.properties bin target - +*.iml +.idea diff --git a/src/evaluation/java/fr/inria/spirals/npefix/resi/NPEFixTemplateEvaluation.java b/src/evaluation/java/fr/inria/spirals/npefix/resi/NPEFixTemplateEvaluation.java index 7e6c239..873cfef 100644 --- a/src/evaluation/java/fr/inria/spirals/npefix/resi/NPEFixTemplateEvaluation.java +++ b/src/evaluation/java/fr/inria/spirals/npefix/resi/NPEFixTemplateEvaluation.java @@ -116,10 +116,11 @@ protected NPEOutput runProject(String name, String source, String test, String[] if (positions.containsKey(name)) { CtElement element = getElementFromPosition(spoon.getFactory().Type().get(positions.get(name).getClassname()), positions.get(name)); if (element != null) { - output.addAll(strategy1(spoon, (CtExpression) element)); - output.addAll(strategy2(spoon, (CtExpression) element)); - output.addAll(strategy3(spoon, (CtExpression) element)); - output.addAll(strategy4(spoon, (CtExpression) element)); + output.addAll(strategy1(spoon, (CtExpression) element, new String[] {source})); + output.addAll(strategy2(spoon, (CtExpression) element, new String[] {source})); + output.addAll(strategy3(spoon, (CtExpression) element, new String[] {source})); + output.addAll(strategy4(spoon, (CtExpression) element, new String[] {source})); + output.addAll(strategy4(spoon, (CtExpression) element, new String[] {source})); } else { Assert.fail("element not found"); } @@ -188,7 +189,7 @@ private List getVariableAccesses(CtElement elementToReplace, CtTyp /** * Replace null element by existing element */ - private NPEOutput strategy1(Launcher launcher, CtExpression element) { + private NPEOutput strategy1(Launcher launcher, CtExpression element, final String[] inputSources) { NPEOutput output = new NPEOutput(); Location location = getLocation(element); @@ -201,7 +202,7 @@ private NPEOutput strategy1(Launcher launcher, CtExpression element) { Decision decision = new Decision<>(new Strat1A(), location, InstanceFactory.fromCtExpression(ctVariableAccess)); - output.addAll(applyAndRunPatch(launcher, new ReplaceLocal(ctVariableAccess), element, decision)); + output.addAll(applyAndRunPatch(launcher, new ReplaceLocal(ctVariableAccess), element, decision, inputSources)); } // in global mode the new element as to be the same type variables = getVariableAccesses(element, element.getType()); @@ -211,7 +212,7 @@ private NPEOutput strategy1(Launcher launcher, CtExpression element) { location, InstanceFactory.fromCtExpression(ctVariableAccess)); output.addAll(applyAndRunPatch(launcher, new ReplaceGlobal(ctVariableAccess), element, - decision)); + decision, inputSources)); } return output; } @@ -219,7 +220,7 @@ private NPEOutput strategy1(Launcher launcher, CtExpression element) { /** * Replace null element by new instance */ - private NPEOutput strategy2(Launcher launcher, CtExpression element) { + private NPEOutput strategy2(Launcher launcher, CtExpression element, final String[] inputSources) { NPEOutput output = new NPEOutput(); Location location = getLocation(element); @@ -233,7 +234,7 @@ private NPEOutput strategy2(Launcher launcher, CtExpression element) { Decision decision = new Decision<>(new Strat2A(), location, InstanceFactory.fromCtExpression(newInstance)); output.addAll(applyAndRunPatch(launcher, new ReplaceLocal(newInstance), element, - decision)); + decision, inputSources)); } // in global mode the new element as to be the same type @@ -245,7 +246,7 @@ private NPEOutput strategy2(Launcher launcher, CtExpression element) { location, InstanceFactory.fromCtExpression(newInstance)); output.addAll(applyAndRunPatch(launcher, new ReplaceGlobal(newInstance), element, - decision)); + decision, inputSources)); } return output; } @@ -253,7 +254,7 @@ private NPEOutput strategy2(Launcher launcher, CtExpression element) { /** * Add check not null around null element */ - private NPEOutput strategy3(Launcher launcher, CtExpression element) { + private NPEOutput strategy3(Launcher launcher, CtExpression element, final String[] inputSources) { NPEOutput output = new NPEOutput(); Location location = getLocation(element); @@ -261,7 +262,7 @@ private NPEOutput strategy3(Launcher launcher, CtExpression element) { Decision decision = new Decision<>(new Strat3(), location, new PrimitiveInstance(false)); output.addAll(applyAndRunPatch(launcher, new SkipLine(), element, - decision)); + decision, inputSources)); return output; } @@ -275,7 +276,7 @@ private Location getLocation(CtExpression element) { /** * Skip method */ - private NPEOutput strategy4(Launcher launcher, CtExpression element) { + private NPEOutput strategy4(Launcher launcher, CtExpression element, final String[] inputSources) { NPEOutput output = new NPEOutput(); CtMethod method = element.getParent(CtMethod.class); @@ -307,7 +308,7 @@ private NPEOutput strategy4(Launcher launcher, CtExpression element) { } Decision decision = new Decision<>(new Strat4(type), location, instance); - output.addAll(applyAndRunPatch(launcher, new SkipMethodReturn(ctVariable), element, decision)); + output.addAll(applyAndRunPatch(launcher, new SkipMethodReturn(ctVariable), element, decision, inputSources)); } } return output; @@ -315,7 +316,7 @@ private NPEOutput strategy4(Launcher launcher, CtExpression element) { private List applyAndRunPatch(Launcher launcher, PatchTemplate patchTemplate, - CtExpression expression, Decision decision) { + CtExpression expression, Decision decision, final String[] inputSources) { // clone the parent of the expression CtType originalType = expression.getParent(CtType.class); CtType cloneType = originalType.clone(); @@ -330,7 +331,7 @@ private List applyAndRunPatch(Launcher launcher, // change the model with the new class originalType.replace(cloneType); - List lapses = executeTest(launcher, decision); + List lapses = executeTest(launcher, decision, inputSources); // restore the original class cloneType.replace(originalType); @@ -343,7 +344,7 @@ private List applyAndRunPatch(Launcher launcher, return Collections.emptyList(); } - private List executeTest(final Launcher launcher, final Decision decision) { + private List executeTest(final Launcher launcher, final Decision decision, final String[] inputSources) { Path tempBinFolder = null; try { tempBinFolder = Files.createTempDirectory("npefix_"); @@ -393,7 +394,7 @@ private List executeTest(final Launcher launcher, final Decision decision continue; } - Lapse lapse = new Lapse(null); + Lapse lapse = new Lapse(null, inputSources); lapse.setTestClassName(testClassName); lapse.setTestName(method); lapse.addDecision(decision); diff --git a/src/main/java/fr/inria/spirals/npefix/main/ExecutionClient.java b/src/main/java/fr/inria/spirals/npefix/main/ExecutionClient.java index c87ffe3..acff359 100644 --- a/src/main/java/fr/inria/spirals/npefix/main/ExecutionClient.java +++ b/src/main/java/fr/inria/spirals/npefix/main/ExecutionClient.java @@ -24,7 +24,7 @@ public class ExecutionClient { public static void main(String[] args) { - ExecutionClient executionClient = new ExecutionClient(args[0], args[1]); + ExecutionClient executionClient = new ExecutionClient(args[0], args[1], new String[0]); Config.CONFIG.setRandomSeed(Integer.parseInt(args[2])); executionClient.run(); } @@ -33,10 +33,12 @@ public static void main(String[] args) { private String testName; private int port = Config.CONFIG.getServerPort(); private String host = Config.CONFIG.getServerHost(); + private String[] inputSources; - public ExecutionClient(String classTestName, String testName) { + public ExecutionClient(String classTestName, String testName, String[] inputSources) { this.classTestName = classTestName; this.testName = testName; + this.inputSources = inputSources; } /** @@ -55,7 +57,7 @@ private Selector getSelector() { private void run() { Selector selector = getSelector(); - Lapse lapse = new Lapse(selector); + Lapse lapse = new Lapse(selector, inputSources); lapse.setTestClassName(classTestName); lapse.setTestName(testName); diff --git a/src/main/java/fr/inria/spirals/npefix/main/all/DefaultRepairStrategy.java b/src/main/java/fr/inria/spirals/npefix/main/all/DefaultRepairStrategy.java index ecee97a..79174c4 100644 --- a/src/main/java/fr/inria/spirals/npefix/main/all/DefaultRepairStrategy.java +++ b/src/main/java/fr/inria/spirals/npefix/main/all/DefaultRepairStrategy.java @@ -35,9 +35,10 @@ */ public class DefaultRepairStrategy implements RepairStrategy { + private String[] inputSources; protected List processors; - public DefaultRepairStrategy() { + public DefaultRepairStrategy(String[] inputSources) { processors = new ArrayList<>(); processors.add(new TernarySplitter());// //processors.add(new IfSplitter()); @@ -53,6 +54,7 @@ public DefaultRepairStrategy() { processors.add(new ConstructorEncapsulation()); processors.add(new VariableFor());// //p.addProcessor(new ArrayRead()); + this.inputSources = inputSources; } @Override @@ -62,7 +64,7 @@ public NPEOutput run(Selector selector, List methodTests) { NPEOutput output = new NPEOutput(); - Lapse lapse = new Lapse(selector); + Lapse lapse = new Lapse(selector, inputSources); final TestRunner testRunner = new TestRunner(); for (int i = 0; i < methodTests.size(); i++) { @@ -101,7 +103,7 @@ public NPEOutput run(Selector selector, List methodTests) { throw new RuntimeException(e); } } - lapse = new Lapse(selector); + lapse = new Lapse(selector, inputSources); CallChecker.enable(); CallChecker.cache.clear(); CallChecker.getDecisions().clear(); diff --git a/src/main/java/fr/inria/spirals/npefix/main/all/Launcher.java b/src/main/java/fr/inria/spirals/npefix/main/all/Launcher.java index 91002fc..331a996 100644 --- a/src/main/java/fr/inria/spirals/npefix/main/all/Launcher.java +++ b/src/main/java/fr/inria/spirals/npefix/main/all/Launcher.java @@ -63,7 +63,7 @@ public class Launcher { private final Logger logger = LoggerFactory.getLogger(Launcher.class); public Launcher(String[] sourcePath, String sourceOutput, String binOutput, String classpath) { - this(sourcePath, sourceOutput, binOutput, classpath, 7, new DefaultRepairStrategy()); + this(sourcePath, sourceOutput, binOutput, classpath, 7, new DefaultRepairStrategy(sourcePath)); } public Launcher(String[] sourcePath, String sourceOutput, String binOutput, String classpath, RepairStrategy repairStrategy) { diff --git a/src/main/java/fr/inria/spirals/npefix/main/all/TryCatchRepairStrategy.java b/src/main/java/fr/inria/spirals/npefix/main/all/TryCatchRepairStrategy.java index 18a1031..83f5f53 100644 --- a/src/main/java/fr/inria/spirals/npefix/main/all/TryCatchRepairStrategy.java +++ b/src/main/java/fr/inria/spirals/npefix/main/all/TryCatchRepairStrategy.java @@ -24,7 +24,8 @@ public class TryCatchRepairStrategy extends DefaultRepairStrategy { private static Selector selector; - public TryCatchRepairStrategy() { + public TryCatchRepairStrategy(String[] inputSources) { + super(inputSources); processors = new ArrayList<>(); processors.add(new CheckNotNull());// processors.add(new ForceNullInit());// diff --git a/src/main/java/fr/inria/spirals/npefix/main/patch/SortPatch.java b/src/main/java/fr/inria/spirals/npefix/main/patch/SortPatch.java index c215f3f..39709ec 100644 --- a/src/main/java/fr/inria/spirals/npefix/main/patch/SortPatch.java +++ b/src/main/java/fr/inria/spirals/npefix/main/patch/SortPatch.java @@ -104,7 +104,7 @@ private static void sortPatchProject(String project, String projectSourcePath, S decisions.add(decision); } - PatchesGenerator patchesGenerator = new PatchesGenerator(decisions, spoon); + PatchesGenerator patchesGenerator = new PatchesGenerator(decisions, spoon, new String[] {projectSourcePath}); String diff = patchesGenerator.getDiff(); System.out.println(diff); ((JSONObject) execution).put("diff", diff); diff --git a/src/main/java/fr/inria/spirals/npefix/patch/generator/PatchesGenerator.java b/src/main/java/fr/inria/spirals/npefix/patch/generator/PatchesGenerator.java index 9975d57..ae0cc90 100644 --- a/src/main/java/fr/inria/spirals/npefix/patch/generator/PatchesGenerator.java +++ b/src/main/java/fr/inria/spirals/npefix/patch/generator/PatchesGenerator.java @@ -25,8 +25,9 @@ public class PatchesGenerator { private List decisions; private Launcher spoon; + private String[] inputSources; - public PatchesGenerator(List decisions, Launcher spoon) { + public PatchesGenerator(List decisions, Launcher spoon, String[] inputSources) { this.decisions = new ArrayList<>(); for (int i = 0; i < decisions.size(); i++) { Decision decision = decisions.get(i); @@ -35,6 +36,7 @@ public PatchesGenerator(List decisions, Launcher spoon) { } } this.spoon = spoon; + this.inputSources = inputSources; } public String getDiff() { @@ -148,8 +150,8 @@ private String createPatchDiff(String className, List decisions) { try { String path = getClassPath(type); diff = Diff.diff(r1, r2, false) - .toUnifiedDiff(path, - path, + .toUnifiedDiff("a" + path, + "b" + path, new StringReader(originalClassContent), new StringReader(classContent), 1); } catch (IOException e) { @@ -162,16 +164,28 @@ private String createPatchDiff(String className, List decisions) { private String getClassPath(CtType type) { String path = type.getPosition().getFile().getPath(); String intersection = null; - Set inputSources = spoon.getModelBuilder().getInputSources(); - for (File inputSource : inputSources) { + for (String inputSource : inputSources) { + if (inputSource.startsWith("./")) { + inputSource = inputSource.substring(2); + } if (intersection == null) { - intersection = inputSource.getPath(); + intersection = inputSource; + } else { + intersection = intersection(intersection, inputSource); + } + } + int indexOfIntersection = path.indexOf(intersection); + + if (indexOfIntersection != -1) { + path = path.substring(indexOfIntersection); + if (!path.startsWith("/")) { + return "/" + path; } else { - intersection = intersection(intersection, inputSource.getPath()); + return path; } + } else { + return null; } - path = path.replace(intersection, ""); - return path; } /** diff --git a/src/main/java/fr/inria/spirals/npefix/resi/context/Lapse.java b/src/main/java/fr/inria/spirals/npefix/resi/context/Lapse.java index 0e4f8a1..62625c5 100644 --- a/src/main/java/fr/inria/spirals/npefix/resi/context/Lapse.java +++ b/src/main/java/fr/inria/spirals/npefix/resi/context/Lapse.java @@ -35,8 +35,9 @@ public class Lapse implements Comparable, Serializable { private Date startDate; private Date endDate; private boolean isFinished = false; + private String[] inputSources; - public Lapse(Selector strategySelector) { + public Lapse(Selector strategySelector, String[] inputSources) { this.uniqueId = currentUniqueId++; this.locations = new HashSet<>(); nbApplication = new HashMap<>(); @@ -45,6 +46,7 @@ public Lapse(Selector strategySelector) { startDate = new Date(); this.strategySelector = strategySelector; metadata.put("seed", Config.CONFIG.getRandomSeed()); + this.inputSources = inputSources; } public void increaseNbApplication(Decision decision) { @@ -141,7 +143,7 @@ public Object getMetadata(String key) { public String toDiff(Launcher spoon) { try { - PatchesGenerator patchesGenerator = new PatchesGenerator(decisions, spoon); + PatchesGenerator patchesGenerator = new PatchesGenerator(decisions, spoon, inputSources); return patchesGenerator.getDiff(); } catch (Exception e) { return null; diff --git a/src/test/java/fr/inria/spirals/npefix/patch/PatchesGeneratorTest.java b/src/test/java/fr/inria/spirals/npefix/patch/PatchesGeneratorTest.java index 7a31862..a1cbb6d 100644 --- a/src/test/java/fr/inria/spirals/npefix/patch/PatchesGeneratorTest.java +++ b/src/test/java/fr/inria/spirals/npefix/patch/PatchesGeneratorTest.java @@ -23,6 +23,8 @@ public class PatchesGeneratorTest { + private static String[] inputResources = new String[] { "/foo/src/main/java", "/foo/src/test/java" }; + @Test public void testGeneratePatchStrat4New() throws Exception { spoon.Launcher launcher = getSpoonLauncher(); @@ -33,14 +35,14 @@ public void testGeneratePatchStrat4New() throws Exception { foo.setValue(new NewInstance(Object.class.getCanonicalName(), new String[0], Collections.emptyList())); foo.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -20,2 +20,5 @@\n" + " String result = \"\";\n" + "+ if (array == null) {\n" @@ -63,15 +65,15 @@ public void testGeneratePatchStrat4Var() throws Exception { field.setValue(new VariableInstance("element")); field.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); lapse.addDecision(field); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -20,2 +20,5 @@\n" + " String result = \"\";\n" + "+ if (array == null) {\n" @@ -97,14 +99,14 @@ public void testGeneratePatchStrat3() throws Exception { foo.setValue(new VariableInstance("result")); foo.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -20,6 +20,8 @@\n" + " String result = \"\";\n" + "- for (String element : array) {\n" @@ -130,14 +132,14 @@ public void testGeneratePatchStrat2B() throws Exception { foo.setValue(new NewArrayInstance(String[].class, Collections.EMPTY_LIST)); foo.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -20,2 +20,5 @@\n" + " String result = \"\";\n" + "+ if (array == null) {\n" @@ -157,14 +159,14 @@ public void testGeneratePatchStrat1A() throws Exception { foo.setValue(new VariableInstance("result")); foo.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -20,7 +20,16 @@\n" + " String result = \"\";\n" + "- for (String element : array) {\n" @@ -198,14 +200,14 @@ public void testGeneratePatchStrat2A() throws Exception { foo.setValue(new NewArrayInstance(String[].class, Collections.EMPTY_LIST)); foo.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -20,7 +20,16 @@\n" + " String result = \"\";\n" + "- for (String element : array) {\n" @@ -243,15 +245,15 @@ public void testMutliPatch() throws Exception { field.setValue(new NewInstance(String.class.getCanonicalName(), new String[0], Collections.EMPTY_LIST)); field.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); lapse.addDecision(field); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -105,3 +105,7 @@\n" + " public void multiDecisionLine() {\n" + "- Arrays.asList(field.toString(), field.toString());\n" @@ -272,14 +274,14 @@ public void testElseIfPatch() throws Exception { foo.setValue(new NewInstance(String.class.getCanonicalName(), new String[0], Collections.EMPTY_LIST)); foo.setUsed(true); - Lapse lapse = new Lapse(new DomSelector()); + Lapse lapse = new Lapse(new DomSelector(), inputResources); lapse.addDecision(foo); String s = lapse.toDiff(launcher); System.out.println(s); Assert.assertEquals("" - + "--- main/java/Foo.java\n" - + "+++ main/java/Foo.java\n" + + "--- a/foo/src/main/java/Foo.java\n" + + "+++ b/foo/src/main/java/Foo.java\n" + "@@ -111,4 +111,12 @@\n" + " \n" + "- } else if (array[0].isEmpty()) {\n" diff --git a/src/test/java/fr/inria/spirals/npefix/transformer/TryCatchRepairModelTest.java b/src/test/java/fr/inria/spirals/npefix/transformer/TryCatchRepairModelTest.java index 1a43791..22ca9a3 100644 --- a/src/test/java/fr/inria/spirals/npefix/transformer/TryCatchRepairModelTest.java +++ b/src/test/java/fr/inria/spirals/npefix/transformer/TryCatchRepairModelTest.java @@ -58,7 +58,8 @@ public void testRepairWithTryCatch() throws Exception { rootPath.getFile() + "/../instrumented", rootPath.getFile() + "", rootPath.getFile(), - new TryCatchRepairStrategy()); + new TryCatchRepairStrategy(new String[]{sourcePath.getFile(), + testPath.getFile()})); launcher.instrument();