CLI should use consistent system-dependent path separators

[3flex]

Expected Behavior

Consistent/expected path separators are used. Standard for Java tools is to use : on *nix and ; on Windows for path separation, and , or (a space) for separation of Ant-style filters.

Current Behavior

Bit of a mixed bag.

• --input, --plugin, --config & --config-resource accept both , and ; as path separators regardless of platform.
• --report is an odd one, since it requires using : as an internal separator between components of the input command (e.g. -r txt:reports/detekt.txt). This caused issues on Windows (now worked around).
• --filters separates on , and ; but is a bit different in scope since the value is typically quoted.

Context

Several issues in the past with Windows related to handling of path separation. Consistency with other tools.

Proposal is:

• --input, --plugin, --config & --config-resource allow path separation using only : on *nix and ; on Windows
• --report is deprecated in its current form. Introduce --report-txt, --report-xml and --report-html which each accept a single path to the desired output file. No parsing of input is required except to convert to a file path.
• --filters accepts either "<filter-1> <filter-2>" (quoted, space-separated), "<filter-1>,<filter-2>" (quoted, comma-separated), <filter-1> <filter-2> (unquoted, space-separated), <filter-1>,<filter-2> (unquoted, comma-separated).

Of course, these conventions will have to be enforced as rules going forward as new parameters are introduced.

Your Environment

• Version used: RC9.2
• Operating System and version: Windows
• Link to your project: N/A

[arturbosch]

Hm, platform specific separators could be confusing if you use both windows and *nix?
Configuring CI will need the right separators hmm.

[3flex]

You need to use / path separator on nix and \ on Windows, so paths are already written differently on the CLI on the two platforms.

[vanniktech]

How about we agree on something that works for everyone? I don't like the idea of checking of which OS you use and depending on that configure the CLI parameters.

Also this probably means another breaking change :(

[3flex]

something that works for everyone

That's the problem... there won't be a single separator that works for everyone (just like the path separator is different on *nix and Windows, so the CLI parameters will always be different on the two platforms anyway).

But, as long as a : character isn't used, it probably doesn't matter. @arturbosch do you want to make a call and either recommend a PR or close the issue as wontfix?

[arturbosch]

Hm if I read the conversation again, I think we should change the separator and introduce a breaking change her :(.
What about we just leave ; as a separator on windows and unix?
So:
--input, --plugin, --config & --config-resource will use just ;
--filters could be either also separated by ; or if jcommander allows us to use --filters <first> <second> ...
For --report an semicolon could look weird -r txt;reports/report.txt, -r txt=reports/report.txt could be an option to remove the workaround. Or if jcommander supports arity (seems it does) we could use -r txt reports/report.txt.
Hardcoding reports to cli parameters is not an option.

[3flex]

That's all doable. JCommander supports variable arity which allows --filters <first> <second> ... and fixed arity for --report txt reports/report.txt which I think looks nice.

As for leaving just ; as the separator (and removing ,), that would probably be fine.

But perhaps this change should be deferred to a detekt 2.0 release, rather than another breaking change before 1.0?

[arturbosch]

Hm I think most users use the gradle plugin and this would "only" affect cli users.
I think I would say go for it now :D, cc @Mauin @vanniktech @schalkms

[vanniktech]

Having a hard time imaging the changes here. Can you give a concrete example of the current configuration and how you want to change it?

[arturbosch]

Actually maybe all of these --input, --plugin, --config & --config-resource --filters should use variadic arity and --report uses fixed arity. this would solve all our problems with the separators?
@vanniktech take a look at @3flex last comment.

[schalkms]

Since this issue became quite stale, what are we going to do about?

[3flex]

I propose that for detekt 2.0 the path separators are updated so they're system-dependent for any CLI options that accept multiple paths. That's the convention used for both javac and kotlinc.

I don't have as strong an opinion on the other options since I use detekt from Gradle so I don't have to worry about any issues with those.

[detekt-ci]

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.