Skip to content
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.

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

Main parameter doesn't support Converter #380

Closed
Khazrak opened this issue Jun 30, 2017 · 8 comments
Closed

Main parameter doesn't support Converter #380

Khazrak opened this issue Jun 30, 2017 · 8 comments

Comments

@Khazrak
Copy link

Khazrak commented Jun 30, 2017

The converter isn't called and I get an Exception:
Exception in thread "main" com.beust.jcommander.ParameterException: Could not invoke null
Reason: Can not set java.nio.file.Path field mypackage.MyCommand.file to java.lang.String

@Parameter(description = "File", converter = PathConverter.class)
private Path file;

(The PathConverter is from JCommander)

If I add a name to the Parameter (like -f) then the converter works like a charm.

Whole class:
MyCommand.txt

@juewe
Copy link
Contributor

juewe commented Aug 24, 2017

Fix wrks for me:
in com.beust.jcommander.JCommander#convertValue

`              ...
               String value = a; // If there's a non-quoted version, prefer that one
                Object convertedValue = value;

                 // Fix
                // Main parameter doesn't support Converter
                // https://github.com/cbeust/jcommander/issues/380
                convertedValue = convertValue(mainParameter.parameterized, mainParameter.parameterized.getType(), null, value);

                Type genericType = mainParameter.parameterized.getGenericType();
                if (genericType instanceof ParameterizedType) {
               ...

`

@jeremysolarz
Copy link
Contributor

Dear @juewe,

Thanks for the fix for issue #380 but your fix breaks the tests ...

java.lang.AssertionError: Lists differ at element [0]: a != [a] expected [a] but found [[a]]
	at org.testng.Assert.fail(Assert.java:94)
	at org.testng.Assert.failNotEquals(Assert.java:513)
	at org.testng.Assert.assertEqualsImpl(Assert.java:135)
	at org.testng.Assert.assertEquals(Assert.java:568)
	at org.testng.Assert.assertEquals(Assert.java:533)
	at com.beust.jcommander.JCommanderTest.listParameters(JCommanderTest.java:457)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:100)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:646)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:811)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1137)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
	at org.testng.TestRunner.privateRun(TestRunner.java:749)
	at org.testng.TestRunner.run(TestRunner.java:603)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:368)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:363)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:321)
	at org.testng.SuiteRunner.run(SuiteRunner.java:270)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1284)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1209)
	at org.testng.TestNG.runSuites(TestNG.java:1124)
	at org.testng.TestNG.run(TestNG.java:1096)
	at org.gradle.api.internal.tasks.testing.testng.TestNGTestClassProcessor.runTests(TestNGTestClassProcessor.java:129)
	at org.gradle.api.internal.tasks.testing.testng.TestNGTestClassProcessor.stop(TestNGTestClassProcessor.java:88)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
	at com.sun.proxy.$Proxy3.stop(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.stop(TestWorker.java:120)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:377)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:54)
	at org.gradle.internal.concurrent.StoppableExecutorImpl$1.run(StoppableExecutorImpl.java:40)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)

Best,

JS

@juewe
Copy link
Contributor

juewe commented Aug 25, 2017

Hi Jeremy,
next try:
Added a check in JCommander.java testing if Converter is specified and is not a NoConverter.
Added a Argument-Class and enhancing JCommanderTest with Testcase for Mainparams with a Converter.

Created a patch (simply delete the ".txt" extension form file.

Hope that helps...

FIX360v2.patch.txt

@acostalima
Copy link

@juewe submit a PR

@cbeust cbeust closed this as completed in a2c687e Aug 27, 2017
cbeust added a commit that referenced this issue Aug 27, 2017
FIX #380 Main parameter doesn't support Converter
@jzwolak
Copy link

jzwolak commented May 20, 2018

This issue was fixed 9 months ago and there hasn't been a release since then... so I ran into the bug, too. @cbeust, will you be making a release?

@cbeust
Copy link
Owner

cbeust commented May 20, 2018

1.74 was released about a month ago:

https://bintray.com/cbeust/maven/jcommander/1.74

@la-costa
Copy link

It looks like the problem was fixed in version 1.74.

th-tran added a commit to th-tran/cipher that referenced this issue Oct 28, 2018
@antonio-rodriges
Copy link

Thanks for adding converter capability to parse the main param:

// Fix
// Main parameter doesn't support Converter
// https://github.com/cbeust/jcommander/issues/380
if (mainParameter.annotation.converter() != null && mainParameter.annotation.converter() != NoConverter.class){
convertedValue = convertValue(mainParameter.parameterized, mainParameter.parameterized.getType(), null, value);
}

However, the code does not work if we provide IStringConverterFactory. A workaround is to specify a dummy converter class for the main parameter that cannot be instantiated.

I noticed the solution above Added a check in JCommander.java testing if Converter is specified and is not a NoConverter.

But the condition

if (mainParameter.annotation.converter() != null && mainParameter.annotation.converter() != NoConverter.class){

prevents using the IStringConverterFactory. It would be good to call convertValue anyway. All in all, if there is no appropriate converter, it would just return the same string

converter = new StringConverter();

However, to avoid breaking tests without the condition, should the if clause be extended to account for IStringConverterFactory?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants