Irida feedback on Inspections / Error Highlighting (as of 3245)

Inspection Identifier Name QuickFix
========================================================
Following is an attempt at an algorithm for a QuickFix for naming convention errors.


Naming Convention Definiation
========================================================
Instead of specifying Naming Convention as:
pattern: m_[a-z][A-Za-z0-9]*

Specify as:

prefix:
firstword: character case, character pattern
addlwords: character case, character pattern
suffix:

where
character case: all_uppercase, all_lowercase, first_letter_uppercase
character pattern: alpha, alphanumeric, alphanumeric_first_letter_alpha
-

EXAMPLES
========================================================

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

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

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

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


Name: mCamelCaseAlphaNumeric:
prefix: m
firstword: first_letter_uppercase, alphanumeric_first_letter_alpha
addlwords: first_letter_uppercase, alphanumeric
separator:
suffix:

Name: LowerCaseWithUnderscores
prefix:
firstword: all_lowercase, alphanumeric_first_letter_alpha
addlwords: all_lowercase, alphanumeric
separator:
suffix:

Name: m_lower_case_with_underscores
prefix: m_
firstword: all_lowercase, alphanumeric_first_letter_alpha
addlwords: all_lowercase, alphanumeric
separator:
suffix:


Name: UPPERCASE_WITH_UNDERSCORES
prefix:
firstword: all_uppercase, alphanumeric_first_letter_alpha
addlwords: all_uppercase, alphanumeric
separator:
suffix:

Inputs:

User selects one of the conventions
as the preferred convention.
User can define other conventions which
they want converted to the perferred convention.


ALGORITHM
========================================================

When an identifier name doesn't match the convention,
we check to see if it matches any of the other defined
conventions. If it does, then we break up the identifier
into it's components: prefix, firstWord, addlWords, Suffix
Then we reconstitute the identifier using the preferred
naming convention, replacing the prefix and suffix and separator
with the ones from the preferred convention, and also making
charcters lowercase or uppercase as necessary.

Example #1:
Preferred Convention: m_cameCaseAlphaNumeric

m_groupList => => m_groupList
m_GroupList => m_ Group List => m_groupList
groupList => group List => m_groupList
GroupList => Group List => m_groupList
__groupList => __ group List => m_groupList
__GroupList => __ Group List => m_groupList
grouplist => grouplist => m_grouplist
mGroupList => m Group List => m_groupList
GROUP_LIST => GROUP LIST => m_groupList

Example #2:
Preferred Convention: m_LowerCaseWithUnderscore,

m_group_list => => m_group_list
group_list => group list => m_group_list
groupList => group List => m_group_list
__groupList => __ group List => m_group_list
mGroupList => m Group List => m_group_list
GROUP_LIST => GROUP LIST => m_group_list

Exampe #3:
Preferred Convention: mCamelCaseAlphaNumeric

mGroupList =>
m_group_list => m_ group list => mGroupList
__groupList => __ group List => mGroupList
GROUP_LIST => GROUP LIST => mGroupList

This algorithm seemes to cover the majority of cases.
The work is also bounded because we will only attempt
to convert names which match one of the other
defined naming conventions. The user is free to
add or delete as many of these as they want.


Suggested Defaults using the above
naming convention definitions:

Class naming convention
2-64 [A-Za-z]* => CameCaseAlpha

Constant naming convention
2-64 * => UPPERCASE_WITH_UNDERSCORES

Instance method naming convention
2-64 [A-Za-z0-9]* => camelCaseAlphaNumeric

Instance variable naming convention
2-64 m_[a-z][A-Za-z0-9]* => m_cameCaseAlphaNumeric

Interface naming convention
2-64 [A-Za-z]* => CameCaseAlpha

Local variable naming convention
1-32 [A-Za-z0-9]* => camelCaseAlphaNumeric

Method parameter naming convention
1-32 [A-Za-z0-9]* => camelCaseAlphaNumeric

static method naming convention
2-64 [A-Za-z0-9]* => camelCaseAlphaNumeric

Static variable naming convention
2-64 s_[a-z][A-za-z0-9]* => s_cameCaseAlphaNumeric

I noticed that we have the ability to turn off inspections on a case-by-case basis, but what if I want to turn them off for an entire file, or possibly an entire package?

I like to run the inspections across the entire project or module, but I have some auto-generated java files created by a lexer (antlr). Lot of warnings pop up for these files, but I'm not going to do anything with them, so I'd like for the inspect code to skip them.

Inspections can be turned off for an entire class (by putting the supression annotation on the class declaration), but not by package or module. I've been asked about that by a couple of people, so it's probably worth submitting a JIRA issue.

--Dave Griffith

Dave,

I coded up the algorithm above. Attached is the source code
and some sample screen shots.

I changed the algorithm such that for each identifier which
doesn't match the preferred convention, we iterate over the
other defined conventions and try to find an exact match
for the identifier. If one is found, then we use that
convention to extract the words form the identifier,
dropping any prefix and word separator, e.g. underscores,
and the convert the words into the proper format for the
preferred convention. If no match is found, then we
reiterate over the all the conventions again and look for a
match ignoring case. If a match is found, then we extract
the words, etc.

It works well for converting between all the common
naming conventions, and is extensible if you allow the
user to add new conventions.

Please let me know if this can be used to implement
the quicfix for the naming inspections. Now that I see
it working, I would really like it in IntelliJ!

Fixed a one problem. Attached are updated sources.

FYI I tried this and it was broken in build 3245.
For example,

/** @noinspection Missorted modifers*/
public class TestSuppression
{
static public void blah() {

}

/** @noinspection Missorted modifers*/
static public void foo() {
}

}

The inspection errors still show up even
with the noinspection annoation on the method
and at the class level.

Someone already filed a bug report:

IDEADEV-581 Suppress inspecton for method broken
Fixed in 3251

I'll try again with build 3251.

I have created the following requests in jira:

IDEA-794 Inspection 'for' loop replacable by 'for each' (J2SDK 5.0 only) does not handle the most most common form of iteration on a collection.
http://www.jetbrains.net/jira/browse/IDEA-794

IDEA-914 Quickfix for Naming Convention Inspections
http://www.jetbrains.net/jira/browse/IDEA-914

IDEA-915 Default Naming Conventions should allows alphanumeric characters and less strict length constraints
http://www.jetbrains.net/jira/browse/IDEA-915

I have been using the ALTSHIFTI to display the error/warning hilighting in the inspection results window, and it works really well. The UI for configuring and running the Inspections is a huge improvement over 4.5.4. Everyone who uses this feature should be really thrilled when they upgrade.

Now, we just need more inspections and quick fixes!

Alex wrote:

Now, we just need more inspections and quick fixes!

Even more?! You're not easily satisfied are you? ;)
Seriously, what other inspections are you thinking of?

Bas

Disregarding global analyses that the current API doesn't support, my TODO list has about twenty inspections left on it. Frankly, some of those are scraping the barrel pretty seriously. It's been a great run, but it really does feel like IG is getting to the end of what local(-ish) static analysis of Java programs can accomplish.

--Dave Griffith

Well if you're looking for more work I have one request that got stuck
in IDEABKL purgatory
http://www.jetbrains.net/jira/browse/IDEABKL-2681
as well as a few bugfixes that I periodically find annoying (in
descending order of annoyance)
http://www.jetbrains.net/jira/browse/IDEABKL-2457
http://www.jetbrains.net/jira/browse/IDEABKL-2509
http://www.jetbrains.net/jira/browse/IDEABKL-1843

Sorry if these are on your list already, I just thought I'd take the
opportunity to ping ...
R

I actually hadn't seen these before, and most of them look very easy to fix. Expect progress on these very soon.

--Dave Griffith

I distinctly remember you saying the same thing about 50-100 inspections ago. :)

Jon

Just the response I was hoping for :)
Thanks,
R

Dave Griffith wrote:

Disregarding global analyses that the current API doesn't support, my TODO list has about twenty inspections left on it. Frankly, some of those are scraping the barrel pretty seriously. It's been a great run, but it really does feel like IG is getting to the end of what local(-ish) static analysis of Java programs can accomplish.

--Dave Griffith

What about inspections for local variable "span" and "live time"? (See
Code Complete, second edition, section 10.4. Excellent book, BTW.)

Unless I missed them, there are no inspections related to these concepts
yet.

The span for a variable is the number of statements between two
consecutive references to the variable; the shorter each span, the better.

The live time of a variable is the number of statements over which the
variable is still "live". An inspection could report when the live time
exceeds a given maximum.


-- Rogerio Liesenfeld

I've submitted a variety of feature requests for JetBrains to implement such inspections, but haven't myself. Partially laziness on my part, partially difficulty. Variable liveness inspections require dataflow analysis. Dataflow analysis is tricky to code, and IDEA already has substantial code for doing dataflow analysis. Liveness inspections would take me far longer to code than them, which is why I haven't done it yet.

--Dave Griffith

I just tried my exmaple again with build 3265,but there are still lots of problems with the @noinspection annotation.

IDEADEV-581 was fixed in 3251. The different in 3265 vs 3245 I see is that the inspection names are different, "MissortedModifiers" instead of "Missorted modifiers".

EXMAPLE_1_BEGIN_________________

Here adding the @noinspection annotation
works at the class level to suppress the
missorted modifiers error hilighting and
also if you run Analyze->Inspect Code


/** @noinspection MissortedModifiers*/
public class TestSuppression
{
static public void blah() {

}

static public void foo() {
}
}
EXAMPLE_2_BEGIN______________________

If I place the @noinspection annotation at
the method level, it doesn't work.

public class TestSuppression
{
static public void blah() {

}

/** @noinspection MissortedModifiers*/
static public void foo() {
}
}

EXAMPLE_3_BEGIN________________________

I started to get really confused when I
suppressed StaticMethodNamingConvention
on the "foo" method. The warning says
"Static method name 'foo' is too short."

But what is strange is that I can suppress it
at the method level, but not the class level,
the exact opposite behavior I saw with
the MissortedModifiers inspection. Wierd stuff.

DOESN"T WORK
/** @noinspection StaticMethodNamingConvention*/
public class TestSuppression
{
static public void blah() {

}

static public void foo() {
}
}

WORKS
public class TestSuppression
{
static public void blah() {

}

/** @noinspection StaticMethodNamingConvention*/
static public void foo() {
}
}

EXAMPLE_4_BEGIN________________________

I was trying to see how to suppress all inspections for
the class. I tried just putting /** @noinspection */
but an Exception occurs.


/** @noinspection */
public class TestSuppression
{
static public void blah() {

}

static public void foo() {
}
}