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.

Sign up for GitHub

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

Jump to bottom

Public API: ignore methods outside of class #1580

Merged
merged 3 commits into from
Oct 29, 2018
Merged

Public API: ignore methods outside of class #1580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1d5e21
Public API: ignore methods outside of class
guwirth Oct 27, 2018
a390d8e
fix amp typo in html
guwirth Oct 28, 2018
a2704c4
fix review issues
guwirth Oct 29, 2018
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
33 changes: 19 additions & 14 deletions cxx-squid/src/main/java/org/sonar/cxx/visitors/AbstractCxxPublicApiVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.sonar.sslr.api.TokenType;
import com.sonar.sslr.api.Trivia;
import com.sonar.sslr.impl.ast.AstXmlPrinter;

import java.util.ArrayList;
import java.util.List;
import org.sonar.api.utils.log.Logger;
Expand All @@ -53,11 +52,10 @@
* </ul>
* <p>
* Public API items are considered documented if they have Doxygen comments.<br>
* Function arguments are not counted since they can be documented in function
* documentation and this visitor does not parse Doxygen comments.<br>
* Function arguments are not counted since they can be documented in function documentation and this visitor does not
* parse Doxygen comments.<br>
* This visitor should be applied only on header files.<br>
* Currently, no filtering is applied using preprocessing directive, e.g
* <code>#define DLLEXPORT</code>.<br>
* Currently, no filtering is applied using preprocessing directive, e.g <code>#define DLLEXPORT</code>.<br>
* <p>
* Limitation: only "in front of the declaration" comments are considered.
*
Expand Down Expand Up @@ -91,12 +89,12 @@ public abstract class AbstractCxxPublicApiVisitor<G extends Grammar> extends Squ
@Override
public void init() {
subscribeTo(
CxxGrammarImpl.classSpecifier,
CxxGrammarImpl.memberDeclaration,
CxxGrammarImpl.functionDefinition,
CxxGrammarImpl.enumSpecifier,
CxxGrammarImpl.initDeclaratorList,
CxxGrammarImpl.aliasDeclaration);
CxxGrammarImpl.classSpecifier,
CxxGrammarImpl.memberDeclaration,
CxxGrammarImpl.functionDefinition,
CxxGrammarImpl.enumSpecifier,
CxxGrammarImpl.initDeclaratorList,
CxxGrammarImpl.aliasDeclaration);
}

@Override
Expand Down Expand Up @@ -503,6 +501,7 @@ private void visitTemplateDeclaration(AstNode templateDeclaration) {

private void visitFunctionDefinition(AstNode functionDef) {
if (isPublicApiMember(functionDef)) {

// filter out deleted and defaulted methods
AstNode functionBodyNode = functionDef
.getFirstChild(CxxGrammarImpl.functionBody);
Expand Down Expand Up @@ -817,14 +816,20 @@ private static boolean isPublicApiMember(AstNode node) {
}
}

// method or function outside of class
if (node.is(CxxGrammarImpl.functionDefinition)) {
// filter out function definitions with nested name specifier: should be documented inside of class
AstNode declarator = node.getFirstChild(CxxGrammarImpl.declarator);
if ((declarator != null) && declarator.hasDescendant(CxxGrammarImpl.nestedNameSpecifier)) {
return false;
}
}

return true;
}
}

/**
* Check if inline Doxygen documentation is attached to the given token at
* specified line
* Check if inline Doxygen documentation is attached to the given token at specified line
*
* @param token the token to inspect
* @param line line of the inlined documentation
Expand Down
8 changes: 3 additions & 5 deletions cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxPublicApiVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,17 @@
*/
package org.sonar.cxx.visitors;

import com.sonar.sslr.api.AstNode;
import com.sonar.sslr.api.Grammar;
import com.sonar.sslr.api.Token;
import java.util.Arrays;
import java.util.List;

import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.api.CxxMetric;
import org.sonar.squidbridge.api.SourceFile;

import com.sonar.sslr.api.AstNode;
import com.sonar.sslr.api.Grammar;
import com.sonar.sslr.api.Token;

/**
* Visitor that counts documented and undocumented API items.<br>
* Following items are counted as public API:
Expand Down
15 changes: 8 additions & 7 deletions cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxPublicApiVisitorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,9 @@
import static org.assertj.core.api.Assertions.assertThat;
import org.assertj.core.groups.Tuple;
import static org.assertj.core.groups.Tuple.tuple;
import static org.mockito.Mockito.when;

import org.fest.assertions.Fail;
import org.junit.Test;
import static org.mockito.Mockito.when;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.cxx.CxxAstScanner;
Expand All @@ -58,6 +57,7 @@ private static String getFileExtension(String fileName) {
}

private class TestPublicApiVisitor extends AbstractCxxPublicApiVisitor<Grammar> {

final Map<String, List<Token>> idCommentMap = new HashMap<>();
boolean checkDoubleIDs = false;

Expand Down Expand Up @@ -95,7 +95,7 @@ private Tuple testFile(String fileName, boolean checkDouble)

if (LOG.isDebugEnabled()) {
LOG.debug("#API: {} UNDOC: {}",
file.getInt(CxxMetric.PUBLIC_API), file.getInt(CxxMetric.PUBLIC_UNDOCUMENTED_API));
file.getInt(CxxMetric.PUBLIC_API), file.getInt(CxxMetric.PUBLIC_UNDOCUMENTED_API));
}

return new Tuple(file.getInt(CxxMetric.PUBLIC_API), file.getInt(CxxMetric.PUBLIC_UNDOCUMENTED_API));
Expand All @@ -104,9 +104,9 @@ private Tuple testFile(String fileName, boolean checkDouble)
@Test
public void test_no_matching_suffix() throws IOException {
CxxFileTester tester = CxxFileTesterHelper.CreateCxxFileTester("src/test/resources/metrics/doxygen_example.h", ".",
"");
"");
CxxLanguage language = CxxFileTesterHelper.mockCxxLanguage();
when(language.getHeaderFileSuffixes()).thenReturn(new String[] { ".hpp" });
when(language.getHeaderFileSuffixes()).thenReturn(new String[]{".hpp"});

SourceFile file = CxxAstScanner.scanSingleFile(tester.cxxFile, tester.sensorContext, language);

Expand All @@ -121,7 +121,7 @@ public void doxygen_example() throws IOException {

@Test
public void to_delete() throws IOException {
assertThat(testFile("src/test/resources/metrics/public_api.h", true)).isEqualTo(tuple(43, 0));
assertThat(testFile("src/test/resources/metrics/public_api.h", true)).isEqualTo(tuple(44, 0));

}

Expand All @@ -132,7 +132,7 @@ public void no_doc() throws IOException {

@Test
public void template() throws IOException {
assertThat(testFile("src/test/resources/metrics/template.h", false)).isEqualTo(tuple(14, 4));
assertThat(testFile("src/test/resources/metrics/template.h", false)).isEqualTo(tuple(13, 4));
}

@Test
Expand Down Expand Up @@ -161,6 +161,7 @@ public void public_api() throws UnsupportedEncodingException, IOException {
final Map<String, String> expectedIdCommentMap = new HashMap<>();

expectedIdCommentMap.put("publicDefinedMethod", "publicDefinedMethod");
expectedIdCommentMap.put("publicDeclaredMethod", "publicDeclaredMethod");
expectedIdCommentMap.put("aliasDeclaration", "aliasDeclaration");
expectedIdCommentMap.put("publicMethod", "publicMethod");
expectedIdCommentMap.put("testStruct", "testStruct");
Expand Down
Loading