SRRS Inspection General and Code->Quicfix/Cleanup

I suggest anyone with an interest in this feature to try "Inspector General"
plugin by sixthandredriver.com software. They have an action 'Rationalize
Control Flow' (bound to CTRLALTQ) which does something along the lines of
what I envisioned for this Code->Quickfix/Cleanup action
(http://www.jetbrains.net/jira/browse/IDEADEV-17781)

The approach of 'Rationalize Control Flow' is that it is an integrated one pass
rewriting of the code, and so it avoids the hurdle faced by this Code->Quickfix/
Cleanup action which has to try to apply many independent (and possible
contradicting or overlapping) quickfixes which could be error prone.

The downside of 'Rationalize Control Flow' is that there is no preview
beforehand. Also, similar to Analyze->Inspect Code, you cannot select a portion
of code or a method to limit the refactoring scope. You have to perform
'Rationalize Control Flow' on the entire file. I guess it is similar to
Code->Reformat Code which also doesn't give you any preview beforehand.
Unlike Code->Reformat, 'Rationalize Control Flow' does not give you minute control
over the control flow normalization it is performing, which I think would be frustrating
if certain transformations are undesired.

So, if you want to cleanup some code in the future, you might do the following sequence:
Code->Reformat Code
Code->Rationalize Control Flow
Code->Quickfix/Cleanup Code

Maybe Code->Rationalize Control Flow should subsume all inspections which
modify the control or data flow, e.g use continue statements to flatten loop
structures, prefer early returns, Scope Too Broad, use JDK 5.0 for each, etc.

And the Code->Quickfix/Cleanup Code could be used for all other quickfixes, e.g.
missorted modifiers, c-style array declaration, redundant field initialization,
Unnecessary 'final' for local variable, Unnecessary 'this' qualifier,
Unnecessary fully qualified name.

I'm not really sure what's the best approach, but I feel that Jetbrains (maybe
with sixthandredriver.com's help) is tantalizing close to really unlocking the
power of all Inspections knowledge/quickfixes accumulated to-date, and taking
it all to a new level.

I've attached one example of the transformation that the 'Rationalize Control
Flow' performed on some code with a single click. Being able to do these
kinds of transformations as easily as you do Code->Reformat is very appealing.
No more having to use the Caveman method and run Inspect->Analyze Code, then
click click click expand the Inspection Results Tree then apply quickfixes one
by one.



Attachment(s):
7274_inspector_general_example_ratioalize_control_flow.png
7 comments
Comment actions Permalink

+The downside of 'Rationalize Control Flow' is that there is no preview
beforehand. Also, similar to Analyze->Inspect Code, you cannot select a portion
of code or a method to limit the refactoring scope.+

Preview and sub-file scope selection are in our current plan for version 2.0.

+
Maybe Code->Rationalize Control Flow should subsume all inspections which
modify the control or data flow, e.g use continue statements to flatten loop
structures, prefer early returns, Scope Too Broad, use JDK 5.0 for each, etc.
+

To the best of my knowlege, it does all of that already. Is the issue just that the InspectorGeneral configuration is separate from the Inspection configuration, or is the issue that the InspectorGeneral configuration is intentionally simplified? InspectorGeneral is by design an opinionated piece of software, with many code transformations bundled together and not independently toggleable. Our view is that many of the transformations of InspectorGeneral should always be applied, as code subject to such transformations will always be clearer.

Note also that InspectorGeneral goes farther than the built-in inspections in two particulars. First, several of InspectorGeneral's rules don't make much sense as inspections, due to having a large number of preconditions and being quite difficult to explain. Second, if applying a rule would enable another rule to be fired, InspectorGeneral will recursively do so, as many times as necessary.

+
And the Code->Quickfix/Cleanup Code could be used for all other quickfixes, e.g.
missorted modifiers, c-style array declaration, redundant field initialization,
Unnecessary 'final' for local variable, Unnecessary 'this' qualifier,
Unnecessary fully qualified name.+

All of these (except for redundant field initialization) are already be available in the "Normalize References and Declarations" action, also available in InspectorGeneral.

Sixth and Red River Software
"Code with Grace and Verve"

0
Comment actions Permalink

I don't really have time to organize my thoughts today, so what follows
is stream of consciousness.
-Alex

--

I'm thinking that the Rationalize Control Flow (RCF) and Normalize References
and Declarations (NRaD) could be part of a single action 'Code->Normalize Code'
One is normalizing the control flow and the other is normalizing references
and declarations, so I think of them as something I would do together.

Mabye in the dialog you could still give a checkbox for RCF and NRaD
but they would be both enabled by default.

--

I think you could just leave all the IDEA inspections as they are, but what you
could do is change the error color for all inspections which are covered by
Code->Normalize Code.

So, for instance, I normally color all warnings in yellow, but for inspection
warnings which Code->Noramlize Code could fix, I could color them in different
color, e.g. light purple. (See picture).

That way, if I see code marked in light purple, then I know I can run
Code->Normalize Code to clear it up quickly.

If the code is marked in yellow, then I know I have to hit ALT+ENTER to access
the quickfix or there may be no quickfix.

I hadn't thought about this before, but really IDEA could have color coded
warnings that have other methods of being fixed, for example 'Raw use of parametized
type' and other warnings that can be addressed by Code->Generify... could be colored
one color. Warnings like "if without braces" that can be fixed by Code->Reformat
could be colored another color.

In another thread we have already been talking about adding different labels
to the Inspection descriptions in Errors dialog so they are searchable, e.g.
"New in 7", "Quickfix Available", etc. Bas has already done this for InspectionGadgets
inspections.

What I would suggest is that in addition to adding 'Quickfix Avaiable", inspections
could be tagged with alternative fix paths 'Fixable by Reformat Code',
'Fixable by Normalize Code', 'Fixable by Generify...', etc.

Then, if someone wanted to do what I was suggesting, they could easily search and
find all the inspections that are 'Fixable by Code->Normalize Code' and then change
the warning/gutter bar color style if they so choose.

--

I could really use that 'Quickfix Avaiable' available tagging right now, because
what I'd like to know is how many quickfixes are available in IDEA. Actually, what
I want to know is how many automated quickfixes are avaiable. I don't care about
quickfixes like 'Generify..' or 'Rename...', etc.

And for Inspector General, the current documentation is so sketchy, I don't really
know what % of IDEA's automated quickfixes it covers.

Also, does it only work for Java, or does it work for JSP, Javascript, etc. ?

--

I do have an issue that the Inspector General configuration is completely separate
from the Errors.. It would be great if these were somehow in sync.

I really like this Code->Nroamlzie Code feature and would want it to grow and
support most of the automated quickfixes, but I imagine as the scope grows, so
will the number of options and the overlap with the Errors.

Couldn't you just add your own section 'Normalization' that contains your unique
options like "use continue statements to flatten loop structures", "prefer early returns".
You can just create dummy inspections for them. You wouldn't highlight anything in
the Editor/gutter, but you would use their on/off state and any options when
Code->Normalize Code is run. These inspections could be marked specially, similar
to how global inspections are marked.

The advantage of doing it this way, is that for areas where there is overlap, you could
reuse the on/off settings of the other inspections.

--

On a totally unrelated subject, after all these talk about tasks in foreground/background, etc.
one thing I just suddenly noticed is that Code->Reformat, and this Code>Rationalize Control Flow
both freeze IDEA when they are running.

It would be nicer if the current Editor tab was disabled, and the status bar showed the progress
of the Reformat or Rationalize Control Flow.

Some of these Rationalize Control Flow I have run have taken minutes to complete. If I have a DUAL
or QUAD core workstation, I could let IDEA/Inspector General work away in the one tab, while I go
and do something else.

I don't suppose Inspector General could be made multi-threaded? What if you are running it on multiple
files; Can you safely have different threads working on different files?

--

I personally would like to see this part of IDEA and not a plugin, because I
think if it becomes what I see it could, then I would use it all the time and
due to it's complexity I feel like it needs to be in IDEA in order to remain
robust during EAPs, since I only ever use the official IDEA version about 1% of
the time, the EAP the other 99% of the time.

I also really like the concept of the Infer Nullity Associations, but I haven't had
anytime to try that out.

But I think this Code->Normalize Code is really a novel, gee whiz, extremely
high value feature. This will evolve the InspectionGadgets technology to to a
higher plane of existence, leaving non-IDEA users back in the Stone Age. :)

I'm really excited about this technology, and if I was Jetbrains Mgmt I'd be
calling up SRRS and asking them what will it take-- I recommend a big sack of
money as a down payment plus consulting contract for IDEA 8.0-- for them to
agree to put this technology into IDEA 8.0 and focus all their efforts to make
it robust, support (almost) all the automated quickfixes, and support all the
different languages Java, JSP, Javascript, etc.



Attachment(s):
7274_inspector_general_example_different_error_colors.png
0
Comment actions Permalink

Another untapped normalization opportunity is the naming of classes, methods, fields, variables.

IDEA has naming convention inspections, but offers no quickfixes other than "Rename..."

I think there is an opportunity to provide a fully automated quickfix, e.g. "Normalize Names"

There are two types of normalization that I'm thinking of:
1. Normalize names to following user selected naming convention for class, method, field, variable, constant/enum.
2. Normalize names within special contexts, e.g.
a. I would like Exceptions to be named e, f, g, h, etc. rather than numberFormatExeption
b. I would like the name of loop counters to be named i, j, k, l, m, etc. rather than index, position, etc.
c. I would like the name of Iterator and Enumeration to be i, j, k, l, m,etc. or e,f,g,h,etc.
This last one is not as much of a problem now because we hae foreach loop.
d. I would like the local variable in the foreach loop to be named in one of two ways:
i. Just use the first letter of the Class, e.g.
List]]> groupL;
for each (g : groupL) {
}
ii. Or just use the class name in lowercase
for each (group : groupL) {
}

There are some special cases, esp. when the class name is long.
I often see loop variables named like this:
for each (aGroup : groupL) {
}
or
for each (anGroup : groupL) {
}
or
for each (anGroupElement : groupL) {
}

I had brought #1 before and even wrote some sample code which automatically
converted names between various naming conventions. See this Jira:

http://www.jetbrains.net/jira/browse/IDEABKL-3160
Quickfix for Naming Convention Inspections

The problem is that how I specified a naming convention was different from IDEA's
naming convention inspections.

For example, I defined a prefix, firstword pattern, addlword pattern, word
separator (e.g. underscore '_'), and suffix.

Here is the definition for camelCase, the most common Java naming convention for variables:

Name: camelCaseAlphaNumeric:
prefix:
firstword: all_lowercase, alphanumeric_first_letter_alpha
addlwords: first_letter_uppercase, alphanumeric
separator:
suffix:

But if you look in IDEA's naming convention inspections, they specify it as:

Pattern:[a-z][A-Za-z\d]*
Min Length: 5 Max Length: 32

The length restrictions aren't that important -- I don't think most people care
about that-- but the key difference is the Regexp vs. more structured
definition above. The problem with the Regexp is that it doesn't distinguish
the words within the name, and that is a problem if you are converting between
different naming conventions.

IDEA's Code Style Code Generation settings have yet another set of naming
convention settings, although they only allow you to specify prefix and suffix
characters for the naming conventions. (See attached screenshot). Someone filed
a Jira about the mismatch between IDEA Code Generation settings and the IDEA
Inspections:

http://www.jetbrains.net/jira/browse/IDEADEV-13077
Naming convention inspections is able to get information from project code style but they don't

http://www.jetbrains.net/jira/browse/IDEADEV-252
"IG: Instance variable naming convention": should (opt.ly) reuse the system pattern info


Anyway, I think there is an opportunity to do some kind of "Normalize Names".
The key thing is it needs to offer automated fixes (w/ preview). Anyone that
has inherited or adapted some existing code with some ugly naming convention
can attest that it's monotonous work trying to Rename all these variables.



Attachment(s):
7291_code_style_code_generation.png
7291_constant_naming_convention_settings.png
7291_instance_naming_convention_settings.png
prototype_mCamelCaseAlphaNumeric.gif
prototype_rename_identifier_select_convention_to_convert_to.gif
0
Comment actions Permalink

I often see code between two if branches which is almost the same code except for a little tweak. See example below.

I can use IntelliJ's Analyze->Locate Duplcate, and it will find the duplicates below -- you have to check Anonymize Literals for it to work -- but the only automated fix it gives me is Extract Method is often not the ideal solution, esp. if the lines of codes are not used anywhere else. Try doing Extract Method on the line below (marked "Line 1") and you'll see it didn't really help much and it didn't even prompt me to automatically replace the other duplicated line. I think the problem is Extract Method is using a hardcoded set of parameters to search for duplicate code fragments in the current file, so if you used any special options ,e.g Anonymize Literals, then Extract Method won't recognize the duplicate code fragments.

I was thinking that another option to Rationalize Control Flow could be to pull up duplicated code inside if branches. That would clean up alot of places in the code I see.
I've seen even worse stuff inside JSP files where it's a mis of java and html.

IDEA itself needs to add a Quickfix in the Locate Duplicates dialog for Extract Method (http://www.jetbrains.net/jira/browse/IDEABKL-2623), but they should also give you a quickfix to Pull Duplicate Code Up within the same method.



package org.intellij;

import java.io.*;

/**

  • User: alex

*/
public class TestLocateDuplicates
{
//
// Locate Duplicates Will identify the two lines marked below as
// duplicated, but they don't give you an option to automatically
// refactor out the dupliated code.
// Extract Method is not the answer.
//
void printSelect(PrintWriter out,String[] contacts,int selected) {
out.println(""); for(int i = 1; i<=contacts.length;i++) { if (i == selected) { // line 1 out.println(" " + contacts+); } else { // line 2 out.println(" " + contacts+); } } out.println(""); } void printSelectDuplicateCodeCollapsed(PrintWriter out, String[] contacts, int selected) { out.println("]]> ");
for (int i = 1; i <= contacts.length; i++) {
String selectedText = (i == selected) ? "selected" : "";
out.println(" <option value='" + i + "' "selectedText"> " + contacts+);
}
out.println("</select>");
}
}

0
Comment actions Permalink

I noticed the code I posted has the brackets missing int he code. I used {/pre} using square brackets not curly brackets, but it didn't preserve the square brackets. I thought that used to work.

I'll create a Jira.

0
Comment actions Permalink

I created a Jira request for a new inspection that would search for duplicate code within methods, and then offer quickfix to collapse the duplicate code either by pulling up duplicate code out of branched code or by rolling duplicate code into a loop if possible

Inspection -- Locate Duplicate Code within Methods, Offer quickfix to pull up / refactor out duplicate code.
http://www.jetbrains.net/jira/browse/IDEA-15225

Since we are constraining the duplicate search, it might not be that expensive and so we could do it all the time as part of the inspections parse, esp. if you have a DUAL or QUAD core workstation.

One related idea I've had is that the inspections parse could be split up into two parts. (See attached picture). You could optionally put a 2nd profile to be run after the 1st inspection profile. The idea is that all the quick and important inspections you put in 1st profile and these will run fast and update fast as you edit the code. Then, you have this 2nd profile which has more expensive checks like this locate duplicates. We no longer have the inspections execution time profiling that Dave put in there temporarily, but if we had that back in there, you could see if there are a few inspections using most of the time, or not.

0

Please sign in to leave a comment.