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

improve public API typedef handling #969

Merged
merged 1 commit into from
Oct 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,53 @@ public class UndocumentedApiCheckTest {
@Test
public void detected() throws UnsupportedEncodingException, IOException {
CxxFileTester tester = CxxFileTesterHelper.CreateCxxFileTester("src/test/resources/checks/UndocumentedApiCheck/no_doc.h", ".");
SourceFile file = CxxAstScanner.scanSingleFile(tester.cxxFile, tester.sensorContext, new UndocumentedApiCheck());
SourceFile file = CxxAstScanner.scanSingleFile(tester.cxxFile, tester.sensorContext, new UndocumentedApiCheck());

checkMessagesVerifier.verify(file.getCheckMessages()).next().atLine(6)
.next().atLine(11).next().atLine(13).next().atLine(14).next()
.atLine(17).next().atLine(19).next().atLine(21).next()
.atLine(23).next().atLine(26).next().atLine(48).next()
.atLine(52).next().atLine(53).next().atLine(56).next()
.atLine(58).next().atLine(60).next().atLine(63).next()
.atLine(65).next().atLine(68).next().atLine(74).next().atLine(77);
checkMessagesVerifier.verify(file.getCheckMessages())
.next().atLine(6) // class
.next().atLine(11) // public method
.next().atLine(13) // enum
.next().atLine(14) // enumerator
.next().atLine(17) // enum value
.next().atLine(19) // member variable
.next().atLine(21) // member variable
.next().atLine(23) // public inline method
.next().atLine(26) // public template method
.next().atLine(29) // protected method
.next().atLine(51) // public member variable
.next().atLine(55) // struct
.next().atLine(56) // struct member
.next().atLine(59) // forward declaration
.next().atLine(61) // fuction declaration
.next().atLine(63) // global emptyEnum
.next().atLine(66) // global testEnum
.next().atLine(68) // global enum value
.next().atLine(71) // global testEnumWithType
.next().atLine(73) // global enum value
.next().atLine(76) // global testScopedEnum
.next().atLine(78) // global enum value
.next().atLine(81) // global union
.next().atLine(87) // template
.next().atLine(90) // function definition
.next().atLine(94) // typedef
.next().atLine(96) // typedef struct
.next().atLine(98) // struct member
.next().atLine(99) // struct member
.next().atLine(102) // typedef class
.next().atLine(105) // class member
.next().atLine(106) // class member
.next().atLine(109) // typedef union
.next().atLine(111) // union member
.next().atLine(112) // union member
.next().atLine(115) // typedef enum
.next().atLine(117) // enum member
.next().atLine(118) // enum member
.next().atLine(121) // typedef enum class
.next().atLine(123) // enum member
.next().atLine(124) // enum member
.next().atLine(127) // class OverrideInClassTest
.next().atLine(138); // struct OverrideInStructTest
//.next().atLine(143); // @todo struct ComplexOverrideTest

for (CheckMessage msg : file.getCheckMessages()) {
assertThat(msg.formatDefaultMessage()).isNotEmpty();
Expand Down
72 changes: 70 additions & 2 deletions cxx-checks/src/test/resources/checks/UndocumentedApiCheck/no_doc.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ class testClass

int inlineCommentedAttr;

void inlinePublicMethod();

void inlinePublicMethod() {};

template <class T>
void templateMethod();

protected:
virtual void protectedMethod();

Expand Down Expand Up @@ -65,6 +68,16 @@ enum testEnum
enum_val
};

enum testEnumWithType : int
{
enum_val
};

enum class testScopedEnum
{
enum_val
};

union testUnion
{

Expand All @@ -77,3 +90,58 @@ struct tmplStruct
void func() {
for (int i = 0; i < 10; i++) {}
}

typedef int int32;

typedef struct
{
int a;
float b;
} typedefStruct;

typedef class
{
public:
int a;
float b;
} typedefClass;

typedef union
{
int a;
float b;
} typedefUnion;

typedef enum
{
A,
B
} typedefEnum;

typedef enum class
{
A,
B
} typedefEnumClass;

class OverrideInClassTest
{
virtual void defaultMethod() override;
public:
virtual void publicMethod() override;
protected:
virtual void protectedMethod() override;
private:
virtual void privateMethod() override;
};

struct OverrideInStructTest
{
virtual void defaultMethod() override;
};

//@todo
//struct ComplexOverrideTest
//{
// virtual AAA::BBB::CCC* AAA_CALL method() const noexcept override;
//};
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import com.sonar.sslr.api.Token;
import com.sonar.sslr.api.Trivia;
import com.sonar.sslr.impl.ast.AstXmlPrinter;
import org.sonar.cxx.preprocessor.SourceCodeProvider;

/**
* Abstract visitor that visits public API items.<br>
Expand All @@ -45,6 +44,7 @@
* <li>classes/structures</li>
* <li>class members (public and protected)</li>
* <li>structure members</li>
* <li>union members</li>
* <li>enumerations</li>
* <li>enumeration values</li>
* <li>typedefs</li>
Expand Down Expand Up @@ -196,6 +196,11 @@ private void visitDeclaratorList(AstNode declaratorList) {
return;
}

// ignore classSpefifier typedefs (classSpefifier at classSpefifier)
if( isClassSpefifierTypedef(declaratorList)) {
return;
}

// ignore friend declarations
if (isFriendDeclaration(declaratorList)) {
return;
Expand All @@ -221,6 +226,26 @@ private void visitDeclaratorList(AstNode declaratorList) {
}
}

private boolean isClassSpefifierTypedef(AstNode declaratorList) {
AstNode simpleDeclSpezifierSeq = declaratorList.getPreviousSibling();
if (simpleDeclSpezifierSeq != null) {
AstNode firstDeclSpecifier = simpleDeclSpezifierSeq.getFirstChild(CxxGrammarImpl.declSpecifier);
if (firstDeclSpecifier != null && firstDeclSpecifier.getToken().getType() == CxxKeyword.TYPEDEF) {
AstNode classSpefifier = firstDeclSpecifier.getNextSibling();
if (classSpefifier != null) {
switch ((CxxKeyword) classSpefifier.getToken().getType()) {
case STRUCT:
case CLASS:
case UNION:
case ENUM:
return true;
}
}
}
}
return false;
}

private boolean isFriendDeclaration(AstNode declaratorList) {
AstNode simpleDeclNode = declaratorList
.getFirstAncestor(CxxGrammarImpl.simpleDeclaration);
Expand Down Expand Up @@ -667,14 +692,15 @@ private static boolean isPublicApiMember(AstNode node) {
if (classSpecifier != null) {

AstNode enclosingSpecifierNode = classSpecifier
.getFirstDescendant(CxxKeyword.STRUCT,
CxxKeyword.CLASS, CxxKeyword.ENUM);
.getFirstDescendant(CxxKeyword.STRUCT, CxxKeyword.CLASS,
CxxKeyword.ENUM, CxxKeyword.UNION);

if (enclosingSpecifierNode != null) {
switch ((CxxKeyword) enclosingSpecifierNode.getToken()
.getType()) {
case STRUCT:
// struct members have public access, thus access level
case UNION:
// struct and union members have public access, thus access level
// is the access level of the enclosing classSpecifier
return isPublicApiMember(classSpecifier);
case CLASS:
Expand Down