Issue Details (XML | Word | Printable)

Key: SONAR-881
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Freddy Mallet
Reporter: Olivier Gaudin
Votes: 1
Watchers: 2
Operations

If you were logged in you would be able to see more operations.
Sonar

Use @Override annotation to avoid counting undocumentedAPI when overriding public API

Created: 03/Jun/09 04:00 AM   Updated: 27/Jan/10 09:55 AM   Resolved: 31/Oct/09 05:41 PM
Component/s: Squid
Affects Version/s: 1.9
Fix Version/s: 1.12

Time Tracking:
Not Specified

Issue Links:
Duplicate
 
Related
 


 Description  « Hide

Until sonar-870 is resolved, a short term solution could be to ignore a public method when it declares @Override



Olivier Gaudin added a comment - 09/Jul/09 04:57 AM

There is no need to do so, as a mechanism exists already with javadoc.
By using /** {@inheritDoc} */, it indicates that doc should be inherited and Squid does not count it as undocumented API anymore.
Closing the ticket


François LEIBER added a comment - 19/Aug/09 05:25 PM

I don't really agree here: methods declaring the @Override annotation automatically inherit the javadoc from their parent.
Therefore, we clearly don't want to pollute our code with unnecessary javadoc lines, but we still wish to have a correct documentation count. Of course, implementing a full #SONAR-870 would be better, but the @Override solution was a good alternative, since this is a widely spread annotation anyone in Java5 should be using.


Olivier Gaudin added a comment - 20/Aug/09 12:43 PM

François, can you point somewhere that documents your first sentence, please ?


François LEIBER added a comment - 07/Sep/09 02:53 AM

According to the javadoc reference : "When a main description, or @return, @param or @throws tag is missing from a method comment, the Javadoc tool copies the corresponding main description or tag comment from the method it overrides or implements (if any)".
Since a method tagged as @Override necessarily has a parent method, it will inherit all missing elements of its javadoc, so it should be removed from the undocumented API.

Of course, as I said previously, the real fix would be to remove all overriding methods from the undocumented count (or even better, don't include them in the % calculation at all, except if they add a documentation when the parent doesn't have one), but depending on your analyzer, handling the @Override annotation could have the benefit of being easy to do, while being correct and solving completely the issue for source codes which use this annotation.


Stephan Schuster added a comment - 21/Sep/09 03:16 AM

I agree with Francois. The squid component should handle the @Override annotation. Using /** {@inheritDoc} */ in these cases is annoying and pollutes the code unnecessarily. This issue should be reopened as it is still not fixed in 1.10.1.

Apart from that Sonar is an excellent tool! Many thanks to the developers, great job!


Oliver Gierke added a comment - 29/Sep/09 05:28 AM

I'd like to join into the discussion by mentioning that @Override is not enought to take a glance at. Using Java 5 as compile base, one is not allowed to use @Override on methods implementing interfaces. Thus you will find a lot of code with no source hint of whether a method is related to a superclass. /** {@inheritDoc} */ also doesn't make it for us, as we either leave out comments at all or use the standard Eclipse way of having an

/*
 * (non Javadoc)
 *
 * @see foo.Bar
 */

on methods overriding or implementijng an interface/superclass method.

Regards,
Ollie


Henno Vermeulen added a comment - 22/Jan/10 09:03 AM

I upgraded to v.1.12 but I still have the problem that Sonar reports "Javadoc Method : Missing a Javadoc comment." on many methods where I use @Override to inherit Javadoc on an interface I implement. The generated Javadoc correctly shows the inherited javadoc.

I think our system has 100% public documented API (we ignored constructors, getters and setters), but Sonar reports only 82.7%.


Freddy Mallet added a comment - 25/Jan/10 01:48 PM

Hi Henno, you're talking about two distinct things here :

  • The checkstyle javadocmethod rule which keep on generating a "Javadoc Method : Missing a Javadoc comment"
  • The Squid engine which reports 82.7% instead of 100% public documented API

Concerning the checkstyle javadocmethod rule, you're right and here is the explanation available on the checkstyle web site :

Javadoc is not required on a method that is tagged with the @Override annotation. However under Java 5 it is not possible to mark a method required for an interface (this was corrected under Java 6). Hence Checkstyle supports using the convention of using a single {@inheritDoc} tag instead of all the other tags.

Concerning the Squid engine, I confirm that the @Overide annotation is taken into account. Could you click on the 82.7% measure to see which files have the most great number of public undocumented API ? Perhaps it could help us understanding what is special on those files.

Thanks
Freddy


Henno Vermeulen added a comment - 25/Jan/10 05:11 PM

You are right, checkstyle reports more missing javadoc violations on methods with @Override than the violations I find when I click the percentage. When I click the "javadoc method" violation I find many violations on @Override tags which I do not find when I click the 82.7% (which is now 84,4% by the way). I also saw a class where we did use {@inheritDoc} next to @Override and here checkstyle did not complain.

When I click the percentage I see that the main culprits (with e.g. many @Overrides but "undocumented API: 11") are classes that implement an external interface, not our own code. One is java.util.Set, the other is from our Spring security dependency. The interesting thing is that the javadoc generated by Maven does indeed NOT have javadoc for these methods! Maybe the Maven javadoc plugin cannot add the source/javadoc to the classpath?

I DID find at least one class which implements an interface of our own with 3 methods, defined in another package. The class uses @Override and I see Public undocumented API: 3, Public API: 4. Its constructor is undocumented. (Why not 4 missing?).

The rest of the violations seem to be missing Javadoc on constructors. We decided that we do not require javadoc on them because they are often very simple (we adjusted checkstyle to ignore missing constructor javadoc).


Freddy Mallet added a comment - 27/Jan/10 09:55 AM

Henno, you can create a JIRA ticket in order to add a new property on the Squid Plugin (Sonar settings page). This boolean property would allow to consider (or not) public constructors as public API.