[NETBEANS-4339] PHP Mess Detector - added option to use custom rules

[KacerCZ]

Add option to select custom rule set file in PHP Mess Detector configuration. Customizer panel is also updated.

https://issues.apache.org/jira/browse/NETBEANS-4339

Before:

After:

[KacerCZ]

@tmysik @junichi11 Please advise/help how to deal with selection of built-in rules (codesize, design, ...). When rule set file is selected, then built-in rules don't have to be selected. Mess detector will work only with rules in file.

Validation of settings was already modified to handle this case. Problem is with selection of built-in rules. It seems that it is not possible for JList to have nothing selected.

[tmysik]

@KacerCZ

What about just disabling the Default Rule Sets field if Rule Set File is selected?

CC @junichi11

[KacerCZ]

@tmysik Default rules can be combined with custom rules in file - see http://phpmd.org/documentation/index.html My intention is to make Default Rule Sets optional, so using only custom rule set would be possible.

[tmysik]

@KacerCZ

I am not sure I understand. So - what about adding a checkbox that would enable Default Rule Sets field? Would that work? :thinking:

[tmysik]

@KacerCZ

BTW looking at the attached images, I guess that the Rule Set File textfield should be as long as the other 2 text fields above (it means that both the Browse... buttons would be aligned and after the new one, there would be a free space).

What do you think?

CC @junichi11

[junichi11]

I would add an empty string or whitespace to the list if that's possible. Or I would change the label to a checkbox as @tmysik wrote.

BTW looking at the attached images, I guess that the Rule Set File textfield should be as long as the other 2 text fields above (it means that both the Browse... buttons would be aligned and after the new one, there would be a free space).

Agree.

[KacerCZ]

@junichi11 I added whitespace to list of built-in rule sets and made sure it does not goes into command parameters. @tmysik I fixed alignment of Browse button as you suggested. Same alignment issue is in configuration for PHPStan, shall I prepare PR to fix it?

Options after changes:

Inspection customization:

[KacerCZ]

Please review.

[tmysik]

UPDATE: Now I see that your change is only the empty item in the Rule Sets field, right? I really need to check yout and run your branch, sorry.

The original comment follows:

@KacerCZ

The last screenshot looks a bit unusually to me, I mean the Enabled checkbox. Try to look at some other dialogs in NetBeans, I would expect more the checkbox directly at Rule Sets label. Or does the checkbox enable te whole form (both Rule Sets as well as Rule Set File)?

(Sorry, did not get time to checkout your branch and try it, will try to do it.)

Thank you.

CC @junichi11

[KacerCZ]

@tmysik Yes, I added empty item to list selection of predefined rules. This empty item is ignored when parameters for Mess detector are created. Valid cases are:

• one or more predefined rule sets and no rule set file
• empty predefined rule set and rule set file
• one or more predefined rule sets and rule set file Invalid case is when there is no predefined rule selected and also no rule set file is used.

The second screenshot is from inspection customization dialog. Enabled checkbox enables running of selected analyzer in given configuration.

1. From main menu select Source > Inspect.
2. In Inspect dialog select Use > Configuration and select Default.
3. Click Manage next to Default.
4. Select Analyzer > Mess Detector - that is the dialog from screenshot

[tmysik]

One quick note - maybe instead of an empty item in the list, we could add something like <ignored> or <unused> or similarly. What do you think?

[junichi11]

One quick note - maybe instead of an empty item in the list, we could add something like or or similarly.

Nice idea :) It's easy to understand, I think.

[junichi11]

BTW, I added the Enabled checkbox before because all analyzers are run when a user customizes a configuration if the checkbox doesn't exist.

[KacerCZ]

I replaced empty string with <none> and updated validation. I added missing listener for rule set file textfield in MessDetectorCustomizerPanel (rule set file was not saved after change).

[tmysik]

@junichi11

Junichi, please, just to be sure - could you test whether it works? I am sorry, I am really busy now and did not get to try it myself :/

@KacerCZ

Well done!

Thank you.

[junichi11]

@tmysik Yes, I'll test it later :)

BTW, We should be able to merge PRs again: https://lists.apache.org/thread.html/r619a782b9e828bd62324817b6ae96f93cfab0dd0ea1f67edcfce25c6%40%3Cdev.netbeans.apache.org%3E

[tmysik]

@junichi11

Thanks a lot! :bow:

[junichi11]

@KacerCZ I can't edit "Rule Set File" of the Configurations dialog.

[KacerCZ]

@junichi11 Fixed enabling/disabling of File controls. Thank you.

[junichi11]

@KacerCZ BTW, You should submit icla because we would like to avoid troubles for the license. see: https://www.apache.org/licenses/contributor-agreements.html

Then, As this case, if example data(e.g. ruleset.xml, example code) for testing are needed, please attach them to the JIRA as much as possible. Otherwise, each reviewer has to create data from scratch.

[KacerCZ]

@junichi11 I sent signed ICLA to ASF.

[junichi11]

I sent signed ICLA to ASF.

Thanks!

Let's merge it after the unused import is fixed.

[junichi11]

@tmysik Could you please merge this if it's OK with you?

[tmysik]

@KacerCZ

Well done! Thanks!

@junichi11

Thank you too!

[KacerCZ]

@tmysik @junichi11 Thank you for your guidance and patience.