njit-jerse / specimin

SPECIfication MINimizer. A different kind of slicer for Java.
MIT License
3 stars 5 forks source link

Preserve methods when argument and parameter types are different #293

Closed theron-wang closed 4 months ago

theron-wang commented 4 months ago

When running on a method which references another method, passing in an argument which has a different type than the parameter type, the output file does not preserve the used method, resulting in unintended behavior. For example, when given this input:

import java.util.ArrayList;
import java.util.List;

class Foo {
    void bar() {
        foo(new ArrayList<?>());
    }
    void foo(List<?> baz) {
    }
}

The output only contains the bar() method, removing the foo(List<?>) method entirely:

import java.util.ArrayList;

class Foo {

    void bar() {
        foo(new ArrayList<>());
    }
}

This PR aims to fix this issue by checking the parameters of each method used within the target method with the imports, so they are not removed prematurely and eventually cause an UnsolvedSymbolException, causing the removal of the method.

I wasn't able to make sure that this didn't cause any unintended changes as I have a Windows computer (the tester does not work on Windows), so if someone could verify that, that would be great.

kelloggm commented 4 months ago

I wasn't able to make sure that this didn't cause any unintended changes as I have a Windows computer (the tester does not work on Windows), so if someone could verify that, that would be great.

Two things:

  1. we have continuous integration setup to run on PRs, so the tests will be run automatically by GitHub
  2. while the Specimin tests won't run natively on Windows, they should run without trouble inside the Windows Subsystem for Linux (commonly abbreviated as WSL), which is pretty easy to install and set up on a Windows machine and gives you a unix-like environment. The reason that the test infrastructure doesn't run natively on Windows is that it relies on the unix diff command, which is available on basically all unix-like systems.

In general when making a PR, it's best practice to include a new test in the PR if you have one. I've added the test that you mentioned in the PR description to this PR.

theron-wang commented 4 months ago

Got it, thanks!