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

Improve documentation - how to add usage of var to check IllegalType? #14775

Open
pjljvandelaar opened this issue Apr 9, 2024 · 6 comments
Open
Labels

Comments

@pjljvandelaar
Copy link

On #8978 (comment) is stated that checkstyle can report
var usage as forbidden.
The provide link is no longer working, yet on https://checkstyle.sourceforge.io/checks/coding/illegaltype.html I am unable to find any example related to the usage of var.

I would like to add var to the default checks of IllegalType.
I assume adding <property name="illegalClassNames" value="var"/> to the IllegalType module will remove the default values.
Is that indeed the case?

If so, how can I add var to illegalClassNames without removing the default value?
Of course, I can repeating the default value, but that has drawbacks:

  • duplication of information and
  • becoming responsible for consistency: I now have to check on each update of checkstyle that the default value did not change.

Could the documentation be improved such that it is clearer how to add illegal class names to the default illegal class names.

Thanks in advance,

Pierre

@rnveach
Copy link
Member

rnveach commented Apr 9, 2024

$ cat TestClass.java
public class TestClass {
    void method() {
var lhmap =
        new LinkedHashMap<Integer, String>();
    }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>
    <property name="severity" value="warning"/>
    <property name="haltOnException" value="false"/>

    <module name="TreeWalker">
 <module name="IllegalType">
      <property name="illegalClassNames" value="var"/>
 </module>
    </module>
</module>

$ java -jar checkstyle-10.14.2-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[WARN] TestClass.java:3:1: Usage of type 'var' is not allowed. [IllegalType]
Audit done.

There is no "add to default" for properties. You would have to either redefine the defaults to add new illegals or create 2 instances of the check (default and specialized).

@romani on approvals

@pjljvandelaar
Copy link
Author

@rnveach Do I understand it correctly that
instead of adding an illegalClassName to the default ones, you propose the following config

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>
    <property name="severity" value="warning"/>
    <property name="haltOnException" value="false"/>

    <module name="TreeWalker">
<module name="IllegalType"/> <!-- default check -->
 <module name="IllegalType"> <!-- usage of var check -->
      <property name="illegalClassNames" value="var"/>
 </module>
    </module>
</module>

to add an additional check to the default check.

@romani
Copy link
Member

romani commented Apr 11, 2024

I would like to add var to the default checks of IllegalType.

no, we need to respect all peoples defaults. Some people like var in some contexts.

If so, how can I add var to illegalClassNames without removing the default value?
Of course, I can repeating the default value, but that has drawbacks

the only option for now. We do not know how to make our configs to allow users to reference default values that are hardcoded in Check, may be in future we will find a way.

Could the documentation be improved such that it is clearer how to add illegal class names to the default illegal class names.

yes, please send PR , all doc is at https://github.com/checkstyle/checkstyle/blob/master/src/xdocs/checks/coding/illegaltype.xml.template , just and one more example with description.

@rnveach
Copy link
Member

rnveach commented Apr 11, 2024

@rnveach Do I understand it correctly that

Yes, this is the same as adding to the default. It is just a forbid class list, so it shouldn't matter how much you split it up as long as each module instance has the other properties configured the same.

@pjljvandelaar
Copy link
Author

I would like to add var to the default checks of IllegalType.

no, we need to respect all peoples defaults. Some people like var in some contexts.

Expressing what you want is always hard.
I want to perform both the default checks and to check for 'var' on my codebase - I did not intend to change the checkstyle defaults for everyone.

I learned from this issue discussion
that this can be most easily realized by just repeating the module IllegalType twice with different settings.
That modules can be repeated was not clear to me from the documentation.
Having an example that repeats the module IllegalType with different settings would have helped me a lot!

@romani
Copy link
Member

romani commented Apr 13, 2024

That modules can be repeated was not clear to me from the documentation.
Having an example that repeats the module IllegalType with different settings would have helped me a lot!

Please improve our doc, I shared with you link to file that need a change.

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

No branches or pull requests

3 participants