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

[core] Support default CLI options and document usage examples with @-files and default files #2087

Open
linusjf opened this issue Oct 28, 2019 · 18 comments
Labels
an:enhancement An improvement on existing features / rules in:cli Affects the PMD Command Line Interface in:documentation Affects the documentation
Milestone

Comments

@linusjf
Copy link

linusjf commented Oct 28, 2019

Affects PMD Version:
All.

Rule:
Not rule specific.

Description:

#2006 (comment)

PMD needs a properties file to configure itself so that it can be run without specifying multiple switches on the command line. These switches should be configurable via the properties file and CLI with CLI overriding the files' values.

Running PMD through:

All.

@adangel adangel changed the title [core] [core] Allow to specify command line options via properties file Oct 28, 2019
@adangel adangel added the an:enhancement An improvement on existing features / rules label Oct 28, 2019
@oowekyala
Copy link
Member

It's already possible though not documented. It's a feature of JCommander:

JCommander supports the @ syntax, which allows you to put all your options into a file and pass this file as parameter

Eg write a file pmd_params with your parameters, one per line, then call pmd with pmd @path/to/pmd_params

@linusjf
Copy link
Author

linusjf commented Oct 31, 2019

This looks promising. My concern is that this is parameter expansion as read from the file and may not allow overriding of switches /options. Does it, i.e., can I use the same parameters on the command line?
e.g., can I do this given that I have -f -g in the file?
java @pmdfile -f -g

I haven't got around to testing JCommander if it does support overriding. The documentation does not make any mention of it. I'm unable to find any CLI component that supports properties files. Would it be worthwhile to put in a request to JCommander to support properties files?

cbeust/jcommander#330

@jsotuyod jsotuyod added the in:cli Affects the PMD Command Line Interface label Nov 8, 2019
@adangel
Copy link
Member

adangel commented Nov 8, 2019

I did a simple test:

This is the content of file pmd.parameters:

-d
pmd-core/src/main/java/net/sourceforge/pmd/cpd
-R
rulesets/java/quickstart.xml
-f
text

So, you specify each parameter in a extra line (also the options to a parameter are in a separate lines).

Then you can run pmd like this:

run.sh pmd @pmd.parameters

However, overriding is not possible - additional parameters are possible, but they are added additionally. Which makes sense, you specify it once via the file and once directly, so in sum - twice:

run.sh pmd @pmd.parameters -f xml
...
Can only specify option -f once.

the thus expanded @ file is added to the other commands (the present docu suggests replacing)

(from cbeust/jcommander#330)
So, this is true.

The question is: Do you need overriding? If you know, that one parameter (say e.g. the format) will vary, then just don't put it in the parameters file...

Btw.: CLI only supports specifying one format - the Ant task allows you to specify multiple formats (e.g. to create with one run a html and xml report). This is basically a lack of the CLI interface that PMD provides: e.g. format is not a List, so can be specified only once. But providing multiple format requires that we know for each format, where to save the report to. This means, that -reportfile must be a List as well. However, then we should maybe do something else, like specifying the format like -f text=report.txt -f xml=report.xml.

Btw.: your example doesn't make sense: -f needs an option (the format) and -g doesn't exist in PMD.

I'd suggest to improve the documentation to show, how @parameter files can be used.

If you have a concrete use case for overriding a specific parameter, then we should do this in a separate issue. It's better to solve a concrete problem than trying to fix unnecessary theoretical problems.

@linusjf
Copy link
Author

linusjf commented Nov 9, 2019 via email

@linusjf
Copy link
Author

linusjf commented Nov 11, 2019

https://github.com/cbeust/jcommander/blob/master/src/main/java/com/beust/jcommander/JCommander.java

https://github.com/cbeust/jcommander/blob/master/src/main/java/com/beust/jcommander/defaultprovider/PropertyFileDefaultProvider.java

https://github.com/cbeust/jcommander/blob/master/src/main/java/com/beust/jcommander/IDefaultProvider.java

https://github.com/cbeust/jcommander/blob/master/src/test/java/com/beust/jcommander/DefaultProviderTest.java

I finally found the time to look at JCommander.

It appears that there is support for providing default values via a properties file.

The relevant sources are listed above.

However, it doesn't appear to support overriding. Does someone want to test this, though? It's the end of day for me here.

The documentation at jcommander.org makes no mention of property files, though.

This invalidates my comment above. Strikethrough?

I've raised an issue on JCommander :
cbeust/jcommander#479

@linusjf
Copy link
Author

linusjf commented Nov 11, 2019

I tested the above for properties. And the property values are not being used.

https://github.com/Fernal73/LearnJava/tree/master/JCommander

I still think this should be part of JCommander's code base, not PMD's. Code can always be contributed to JCommander.

@linusjf
Copy link
Author

linusjf commented Nov 11, 2019

As far as I can see, there have been no commits on JCommander for the last three months. Has the project been abandoned?

@linusjf
Copy link
Author

linusjf commented Nov 12, 2019

Another option would be to check out picocli that Checkstyle uses.

https://picocli.info/#_default_provider

https://picocli.info/#_overwriting_single_options

Is it superior?

While it's still my opinion that Picocli and JCommander provide support for properties files fully, PMD might need to implement some of that functionality if it's not provided out-of-the-box.

remkop/picocli#542

@adangel
Copy link
Member

adangel commented Nov 22, 2019

There is nothing preventing us from using an alternative for JCommander - it's just an implementation detail and opaque to the user.

But I'm still missing the use case... Why do we need that? Would it make PMD more complicated to use? E.g. if we allow overriding options, how easy/difficult is it to know, which options are now really used? Wouldn't that lead to other bugs, like "this option is not used", then we dig into it, and see, it is not used, because it is overridden later on...

@linusjf
Copy link
Author

linusjf commented Nov 22, 2019 via email

@oowekyala
Copy link
Member

Another option would be to check out picocli that Checkstyle uses.

IMO PicoCLI looks in general a bit more mature/feature-complete than JCommander. IIRC a really nice feature that JCommander does not do is generate autocompletion files for (at least) Bash. It's been a while since I looked into it though

especially if I'm using a script that calls the java command

This isn't clear to me. Is your script calling PMD directly using the java command, eg java $PMD_MAIN_CLASS?

Why don't you just copy the command-line and manually change options? It looks like you don't need to do this often, and that would work.

I don't wish to create another script that simply accepts "$@" which is
simply a wrapper for the java command line class.

PMD's distribution already includes such scripts

if we allow overriding options, how easy/difficult is it to know, which options are now really used?

I think you're right that overridable options can be abused... but I would also think this is not directly PMD's concern, so does not really warrant emitting warnings. Though I'm pretty sure someone will cut themselves on it one day and feature-request that...

One other "problem" with overridable options is also that every switch should be negatable. For now we have default behaviour, and switches that enable the opposite of the default. There's no switch to go back to default behaviour. Eg if your default profile includes -debug, you can't override it with eg -no-debug later on.

It looks like allowing overriding CLI options would require a significant effort to implement, and especially to test. I'm not sure the effort is proportionate to the current use-case.

@linusjf
Copy link
Author

linusjf commented Nov 22, 2019 via email

@linusjf
Copy link
Author

linusjf commented Nov 25, 2019

@remkop
Copy link

remkop commented May 12, 2020

Hello all,

picocli has built-in support for a PropertiesDefaultProvider that I believe meets @Fernal73's requirements.

This is different from the @-file syntax: an option specified in an @-file and on the command line is applied twice, which may cause issues. By contrast, the default provider only provides a value if an option is not specified on the command line.

If the properties file does not exist, then the default values defined in the applications are applied. This gives end users a way to configure their own default values for the application's options. Users are free to use this or ignore this; this is simply a nice extra feature that mature CLI apps can provide to their users.

One other "problem" with overridable options is also that every switch should be negatable. For now we have default behaviour, and switches that enable the opposite of the default. There's no switch to go back to default behaviour. Eg if your default profile includes -debug, you can't override it with eg -no-debug later on.

I believe this is a side-effect of using @-files as a user-configurable source or default values. Picocli default providers do not suffer from this drawback. If the default value for --debug is false, then you could specify --no-debug on the command line and the command line value would be applied. (If you have many flags like this you may also be interested in picocli's support for negatable options.)

I hope this is useful.

@linusjf
Copy link
Author

linusjf commented May 12, 2020

The decision to use picocli or not rests with the repo maintainers.

I think it's a good idea and can be migrated to in release 7.0.0.

Ps: I can't recall why I didn't like negatable options. Perhaps, it had to do with PMD only having two such options. Otherwise, they're pretty good to have.

@adangel
Copy link
Member

adangel commented Jun 4, 2020

Just FYI - the java command line supports this as well: https://docs.oracle.com/en/java/javase/14/docs/specs/man/java.html#java-command-line-argument-files

@remkop
Copy link

remkop commented Jun 4, 2020

@adangel, regarding support for @-files (argument files): picocli also supports @-files, but looking at the discussion above, I believe picocli's support for a property-file based default provider may meet @Fernal73's requirements better.

Specifically, this supports overriding default values on the command line out of the box.

@adangel adangel modified the milestones: 7.0.0, 7.x Jan 23, 2023
@adangel
Copy link
Member

adangel commented May 2, 2024

Since PMD 7.0.0 uses now picocli, the externalization of the parameters in a file should be possible: https://picocli.info/#AtFiles

All that is missing is an example in the documentation.

Update: we should also use https://picocli.info/#_propertiesdefaultprovider and also provide an example.

@adangel adangel added in:documentation Affects the documentation and removed an:enhancement An improvement on existing features / rules labels May 2, 2024
@adangel adangel changed the title [core] Allow to specify command line options via properties file [core] Document usage example for @-file for specifing command line options May 2, 2024
@adangel adangel changed the title [core] Document usage example for @-file for specifing command line options [core] Document usage example with @-file for specifing command line options May 2, 2024
@adangel adangel added the an:enhancement An improvement on existing features / rules label May 2, 2024
@adangel adangel changed the title [core] Document usage example with @-file for specifing command line options [core] Support default CLI options and document usage examples with @-files and default files May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
an:enhancement An improvement on existing features / rules in:cli Affects the PMD Command Line Interface in:documentation Affects the documentation
Projects
None yet
Development

No branches or pull requests

5 participants