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

API check: Method defined in header file, but out of class, marked as undocumented #1544

Closed
jspam opened this issue Aug 24, 2018 · 7 comments
Assignees
Milestone

Comments

@jspam
Copy link

jspam commented Aug 24, 2018

Description

A method that is declared and documented inside a class, but defined outside the class in the header file, is marked as undocumented API.

class A {
public:
  /** Documented */
  void f();
}

// wrongly marked as undocumented
inline void A::f() {}

PR #1543 contains an example of such a definition.

In Doxygen, the documentation is correctly taken from the definition inside the class.

Steps to reproduce the problem

Analyze public_api.h from #1543 with sonar-cxx

Expected behavior

The method definition in line 111 (inline void testClass::publicDeclaredMethod() {) is not marked as undocumented.

Actual behavior

The method definition in line 111 is marked as undocumented (Undocumented API: testClass).

LOG file

...
06:45:44.554 DEBUG - node: declaratorId line: 108 id: 'inlineCommentedLastAttr' documented: true
06:45:44.555 DEBUG - ***** Node: functionDefinition tokenValue='inline' tokenLine=111 tokenColumn=0
06:45:44.556 DEBUG - Public API: testClass
06:45:44.557 DEBUG - node: declaratorId line: 111 id: 'testClass' documented: false
06:45:44.557 DEBUG - ***** Node: functionDefinition tokenValue='inline' tokenLine=111 tokenColumn=0
06:45:44.558 DEBUG - Public API: testClass
06:45:44.559 DEBUG - node: declaratorId line: 111 id: 'testClass' documented: false
06:45:44.560 DEBUG - ***** Node: classSpecifier tokenValue='struct' tokenLine=117 tokenColumn=0
06:45:44.561 DEBUG - Doc: /// testStruct doc
06:45:44.562 DEBUG - Public API: testStruct
06:45:44.563 DEBUG - node: className line: 117 id: 'testStruct' documented: true
06:45:44.563 DEBUG - ***** Node: classSpecifier tokenValue='struct' tokenLine=117 tokenColumn=0
06:45:44.564 DEBUG - Doc: /// testStruct doc
...

Related information

  • cxx plugin version? 1.1.0
  • SonarQube version? 6.7.4.38452
@guwirth guwirth changed the title Method defined in header file, but out of class, marked as undocumented API check: Method defined in header file, but out of class, marked as undocumented Aug 24, 2018
@guwirth
Copy link
Collaborator

guwirth commented Aug 24, 2018

Hi @jspam,

thanks for providing this sample.

I see two possible solutions:

  • The easy one: ignore all function/methods marked "inline". Think will ignore too much?
  • The better one: create a symbol table and mark already documented functions. Is more complicated because complete symbol name with all namespaces must take into account.

Regards,

@jspam
Copy link
Author

jspam commented Aug 24, 2018

I think ignoring all inline methods will ignore too much. You can mark the definition inside the class as inline without the compiler complaining. The following code would then pass the check although it shouldn't:

class A {
public:
  inline void f();
}

inline void A::f() {}

A symbol table would of course be a more elegant solution, but I agree it is more complicated, especially since you have to take into account using directives as well and basically do a name lookup for all functions you encounter.

A simple approach could be to ignore a function definition with a qualified name that is outside the class or namespace it belongs to. (Ignoring just function definitions with a qualified name would probably work too, but GCC accepts class A { public: void A::f(); } with -fpermissive)

The drawback is that the following code would still be marked as undocumented although Doxygen accepts it. However, the code is already marked as undocumented at the moment and I guess it is more common to document a function at the definition rather than at the declaration.

class A {
public:
  void f();
}

/** Documented */
inline void A::f() {}

@ivangalkin
Copy link
Contributor

@guwirth I don't want to be unhumble, but I already provided a working implementation for the symbol table (#1479) and it could be easily adapted for the improvement of public API check/metric.

@guwirth
Copy link
Collaborator

guwirth commented Aug 26, 2018

I already provided a working implementation for the symbol table (#1479)

@ivangalkin I also tend to use your symbol table implementation. Can you rebase it please.

@ivangalkin
Copy link
Contributor

@guwirth done

@guwirth guwirth closed this as completed Aug 27, 2018
@guwirth
Copy link
Collaborator

guwirth commented Aug 27, 2018

@jspam a possible "fast" workaround could be

class A {
public:
  /** Documented */
  void f();
}

/** @copydoc A::f */
inline void A::f() {}

@guwirth guwirth reopened this Aug 27, 2018
@guwirth guwirth self-assigned this Oct 27, 2018
@guwirth guwirth added this to the 1.2 milestone Oct 27, 2018
@guwirth
Copy link
Collaborator

guwirth commented Oct 29, 2018

closed, see #1580

@guwirth guwirth closed this as completed Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants