On 09/02/18 2:47 PM, Dominique Devienne wrote:
On Fri, Feb 9, 2018 at 10:06 AM, Jaikiran Pai <jai.forums2...@gmail.com>

I need some inputs on how we should go about this specific change/test?
Should this test continue to expect a exception or is it fine to expect
that target to complete cleanly (without copying anything)?

What's the source of the NPE?
If it's an implementation detail, then it's fine to no longer throw IMHO.
We have FileNameMapper interface which has a:

String[] mapFileName(String sourceFileName);

API. The contract/javadoc of this API says:

     * Returns an array containing the target filename(s) for the
     * given source file.
     * <p>if the given rule doesn't apply to the source file,
     * implementation must return null....

We have an implementation of this interface, the IdentityMapper, whose implementation so far, IMO, wasn't following this contract. If it was passed a null source file name, that implementation would return an String[] with one element and the contained element in that array would be null.

Callers of this API are expected to understand that the API itself might return null, so they used to just check the return value and not its contents. They would then go and start working on these individual elements in the array and start running into NPEs in a bunch of places.

The change I made was to the implementation of IdentityMapper, so that it returns null instead of an array with one null element, when the input to it is null. That way, the callers' logic of checking the return value for null, is enough for them to skip such resources.

So to me, this does look like something that we should take care off in that specific test, so that it no longer expects a build exception to be thrown.


To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Reply via email to