Pmd: [java] UnusedFormalParameter and UnusedAssignment can ignore main method

[Fernal73]

Affects PMD Version:

6.29.0

Description:

Rules UnusedAssignment and UnusedFormalParameter flag the args parameter in the main method when it is unreferenced.
It should be ignorable via a parameter.

@oowekyala:

With UnusedAssignment you can also rename your parameter to ignored. We can make #2838 apply to UnusedFormalParameter too. Then there would be a simple way to ignore the warning, and the fact that the parameter is ignored on purpose is explicit (this is the same kind of issue as #2893).

@Fernal73:

That's fine by me. I'd like to be able to retain the semantics of the parameter by prefixing the parameter name with underscore or argsIgnored or argsUnused.

Code Sample demonstrating the issue:

package jls;

import java.util.Objects;

public final class UnsafeVarargs<T> {

  private UnsafeVarargs() {
    throw new IllegalStateException("Private constructor.");
  }

  public static <T> T[] unsafe(T... elements) {
    // unsafe! don't ever return a parameterized varargs array
    return elements;
  }

  public static <T> T[] broken(T seed) {
    // broken! This will be an Object[] no matter what T is
    T[] plant = unsafe(seed, seed, seed);
    return plant;
  }

  public static String[] plant() {
    // ClassCastException
    String[] plants = broken("seed");
    return plants;
  }

  @SuppressWarnings("PMD.SystemPrintln")
  public static void main(String... args) {
    System.out.println(Objects.toString(plant()));
  }
}

Running PMD through: [CLI | Ant | Maven | Gradle | Designer | Other]

[numeralnathan]

I think it is a problem if the code does not pay attention to args. In this case, the program does not care what the args has. However, the program should still check that args.length == 0 and log an error to the user so they can know that no arguments are expected.

[Fernal73]

I think it is a problem if the code does not pay attention to args. In this case, the program does not care what the args has. However, the program should still check that args.length == 0 and log an error to the user so they can know that no arguments are expected.

There are two ways to look at it. Either you write your main body always so that args is referenced whether it's expected to be used or not. Or you don't. If it's the former, you'll never have the parameter flagged. If it's the latter, it will be. Hence, the parameter to mark it as ignorable or not. I'm inclined to go with the former but others might not like the overhead of having to reference the args parameter when it's never going to be used.

[adangel]

For anyone coming along this issue:

• UnusedAssignment reports "args" only if the property "reportUnusedVariables" is set to true.
• UnusedFormalParameters reports "args" only if the property "checkAll" is set to true.

The rules are technically correct and I tend to agree with @numeralnathan .
Since it is a main method, I don't think, you have hundreds of those in your project (which would indicate another problem). I actually like to have this flagged as unused, because as Nathan described, your program should maybe pay attention to args.

[Fernal73]

My programs are mostly sample programs that may or may not use args. Mostly
not.

If I were actually writing a command line utility, then yes, in all
certainty, args would definitely be utilized and we would never argue about
this rule.

By that extension of logic, PMD is to be used only by programmers who are
actually building something and is not meant for learners or enthusiasts.
If that's the case, then it's fine to not amend the rule in the way I
specified.

If however, it's not , then I don't see any harm in providing that
functionality.

Are you expecting sample programs to have logging enabled in their
programs? I guess, that's one way of using the args parameter---log its
contents always.

[oowekyala]

By that extension of logic, PMD is to be used only by programmers who are actually building something and is not meant for learners or enthusiasts.

I think the use case you're describing is really rare, and in their current state, those rules provides value to a learner or enthusiast, by reminding them that ignoring command-line arguments is not a good way to handle them. I don't think your painting it as if we're ignoring a large subset of users is fair.

Are you expecting sample programs to have logging enabled in their programs?

I wouldn't expect PMD to be used on "sample programs" at all

[Fernal73]

main is a special method in java and is needed to execute any command-line program. Had it been an ordinary class or instance method, I'd agree. But main can and should be given special dispensation.

[oowekyala]

There's usually very few main methods per project, meaning the overhead of writing if (args.length == 0) throw ...; is very low

Edit:

In this quote you actually agree that it's good practice:

If I were actually writing a command line utility, then yes, in all
certainty, args would definitely be utilized and we would never argue about
this rule.

[Fernal73]

https://stackoverflow.com/questions/10783190/why-does-main-method-in-java-always-need-arguments

This is an interesting post about main and why it can't have more than one signature. If we had a main() method with no parameters, this argument would be rendered moot.

[Fernal73]

If I were actually writing a command line utility, then yes, in all
certainty, args would definitely be utilized and we would never argue about
this rule.

Yes, it's certainly a good practice. But I'd simply log my parameters---always. LOG.info or lower.

However, not everyone wishes to bother with that overhead.

[Fernal73]

What if you have a command-line utility that has all its parameters read through a properties file?

[oowekyala]

What if you have a command-line utility that has all its parameters read through a properties file?

Log the parameters, or check that there are none, but don't let the parameter go unused... You agreed yourself that it's the good practice... We're going in circles here

[Fernal73]

I still think it should be optionally ignorable. Or do you wish to annotate
the parameter as being unused? That would work as well.

[Fernal73]

https://rules.sonarsource.com/java/tag/unused/RSPEC-1172

"Exceptions
The rule will not raise issues for unused parameters:

that are annotated with
1) @javax.enterprise.event.Observes
2) in overrides and implementation methods
3) in interface default methods
4) in non-private methods that only throw or that have empty bodies
5) in annotated methods, unless the annotation is @SuppressWarning("unchecked") or @SuppressWarning("rawtypes"), in which case the annotation will be ignored
6) in overridable methods (non-final, or not member of a final class, non-static, non-private), if the parameter is documented with a proper javadoc."

Or we could suppress the warning on the parameter with @SuppressWarnings("unused"). This, of course, suppresses the rule.

Or we could allow a custom annotation ( say Unused).

Or simply add main as an exception as well. Optionally. In some ways, main is an 'overridable' method.

[Fernal73]

https://dzone.com/articles/get-warnings-about-unused

In this case, if the javadoc annotated the parameter with @param, the warnings are suppressed or ignored.

[oowekyala]

With UnusedAssignment you can also rename your parameter to ignored. We can make #2838 apply to UnusedFormalParameter too. Then there would be a simple way to ignore the warning, and the fact that the parameter is ignored on purpose is explicit (this is the same kind of issue as #2893).

[Fernal73]

With UnusedAssignment you can also rename your parameter to ignored. We can make #2838 apply to UnusedFormalParameter too. Then there would be a simple way to ignore the warning, and the fact that the parameter is ignored on purpose is explicit (this is the same kind of issue as #2893).

I'm not comfortable with using just ignored. argsIgnored or _args would be more appropriate. Annotating the argument would be best. But if we used a custom annotation such as Unused, PMD would be setting a standard how to handle unused arguments.

I'd suggest an Ignored annotation but that's too similar to JUnit's @Ignore.

[oowekyala]

I think if we want to go the annotation route the only reasonable behaviour is @SuppressWarnings("unused"), and not a custom annotation. Which is already supported.

[Fernal73]

Yes, but that lends itself to some ambiguity. Am I suppressing the warning
or marking the parameter as unused?

The next best thing is to use argsIgnored to retain the semantics of the
parameter. The parameter could possibly be used later.

[Fernal73]

I've updated the bug description in OP and the heading.

Are we satisfied that the rules satisfy the Sonar requirements?
https://github.com/pmd/pmd/issues/2923#issuecomment-732458097

[Fernal73]

I think the use case you're describing is really rare, and in their current state, those rules provides value to a learner or enthusiast, by reminding them that ignoring command-line arguments is not a good way to handle them. I don't think your painting it as if we're ignoring a large subset of users is fair.

I'm not painting you or PMD in any way. I simply state the obvious. That sample or example programs can and will ignore main parameters when they don't need input from the command-line.

Are you expecting sample programs to have logging enabled in their programs?

I wouldn't expect PMD to be used on "sample programs" at all

That may as well be, given that sample programs mostly reside in API docs and tutorials. That's beside the point. You can never predict when or where PMD may be used.

[oowekyala]

Yes, but that lends itself to some ambiguity. Am I suppressing the warning or marking the parameter as unused?

It seems to me that these two things... are the same thing? Why would you do one without the other? It doesn't seem ambiguous to me.

Anyway it looks like we agree that suppressing the violation is enough, and I think the way you've edited the top comment, this issue can be folded into #2838.

Please anyone, protest if I've closed this too early

Are we satisfied that the rules satisfy the Sonar requirements? #2923 (comment)

Please open another issue about that, this thread is already long enough

[Fernal73]

Are we satisfied that the rules satisfy the Sonar requirements? #2923 (comment)

Please open another issue about that, this thread is already long enough

Since you're the implementor, you can tell me whether you are covering any or all the scenarios. I would have to test them out myself. I think we can safely assume @Observable is not covered.