Add undefined function rule

colobot · Sep 29, 2015 · 4ca6440 · 4ca6440
1 parent 8be7e6e
commit 4ca6440
Show file tree

Hide file tree

Showing 11 changed files with 398 additions and 14 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -75,6 +75,7 @@ set(COLOBOT_LINT_SRCS
     Rules/PossibleForwardDeclarationRule.cpp
     Rules/RulesFactory.cpp
     Rules/TodoRule.cpp
+    Rules/UndefinedFunctionRule.cpp
     Rules/UninitializedFieldRule.cpp
     Rules/UninitializedLocalVariableRule.cpp
     Rules/UnusedForwardDeclarationRule.cpp

diff --git a/Common/Context.h b/Common/Context.h
@@ -1,10 +1,13 @@
 #pragma once
 
 #include "Common/ExclusionZone.h"
+#include "Common/FunctionDefinitionContext.h"
 #include "Common/OutputPrinter.h"
+#include "Common/SourceFileInfo.h"
 
 #include <set>
 #include <string>
+#include <unordered_map>
 #include <unordered_set>
 
 class SourceLocationHelper;
@@ -45,6 +48,9 @@ struct Context
 
     std::unordered_set<std::string> reportedOldStyleFunctions;
 
+    std::unordered_set<std::string> definedFunctions;
+    std::unordered_map<std::string, SourceFileInfo> undefinedFunctions;
+
     SourceLocationHelper& sourceLocationHelper;
 
     const std::unique_ptr<OutputPrinter> outputPrinter;

diff --git a/Common/FunctionDefinitionContext.h b/Common/FunctionDefinitionContext.h
@@ -0,0 +1,10 @@
+#pragma once
+
+#include <set>
+#include <string>
+
+struct FunctionDefinitionContext
+{
+    std::set<std::string> definedFunctions;
+    std::set<std::string> undefinedFunctions;
+};
diff --git a/Common/OutputPrinter.cpp b/Common/OutputPrinter.cpp
@@ -30,13 +30,14 @@ class PlainTextOutputPrinter : public OutputPrinter
                         const std::string& destination,
                         const std::string& options = "") override;
 
+protected:
     void PrintRuleViolationImpl(const std::string& ruleName,
                                 Severity severity,
                                 const std::string& description,
                                 const std::string& fileName,
                                 int lineNumber) override;
 
-    void Save() override;
+    void SaveImpl() override;
 
 private:
     std::ofstream m_outputFileStream;
@@ -53,13 +54,14 @@ class XmlOutputPrinter : public OutputPrinter
                         const std::string& destination,
                         const std::string& options = "") override;
 
+protected:
     void PrintRuleViolationImpl(const std::string& ruleName,
                                 Severity severity,
                                 const std::string& description,
                                 const std::string& fileName,
                                 int lineNumber) override;
 
-    void Save() override;
+    void SaveImpl() override;
 
 private:
     void Init();
@@ -80,13 +82,14 @@ class DotGraphOutputPrinter : public OutputPrinter
                         const std::string& destination,
                         const std::string& options = "") override;
 
+protected:
     void PrintRuleViolationImpl(const std::string& ruleName,
                                 Severity severity,
                                 const std::string& description,
                                 const std::string& fileName,
                                 int lineNumber) override;
 
-    void Save() override;
+    void SaveImpl() override;
 
 private:
     void Save(std::ostream& ouputStream);
@@ -124,6 +127,27 @@ class DotGraphOutputPrinter : public OutputPrinter
 } // anonymous namespace
 
 
+struct OutputPrinter::RuleViolationInfo
+{
+    std::string ruleName;
+    Severity severity;
+    std::string description;
+    std::string fileName;
+    int lineNumber;
+
+    RuleViolationInfo(std::string ruleName,
+                      Severity severity,
+                      std::string description,
+                      std::string fileName,
+                      int lineNumber)
+        : ruleName(ruleName),
+          severity(severity),
+          description(description),
+          fileName(fileName),
+          lineNumber(lineNumber)
+    {}
+};
+
 
 OutputPrinter::OutputPrinter(const std::string& outputFileName,
                              std::vector<OutputFilter> outputFilters)
@@ -151,26 +175,40 @@ void OutputPrinter::PrintRuleViolation(const std::string& ruleName,
                                        const std::string& description,
                                        SourceLocation location,
                                        SourceManager& sourceManager,
-                                       int lineOffset)
+                                       int lineOffset,
+                                       bool tentative)
 {
     std::string fileName = GetCleanFilename(location, sourceManager);
     int lineNumber = sourceManager.getPresumedLineNumber(location) + lineOffset;
 
-    PrintRuleViolation(ruleName, severity, description, fileName, lineNumber);
+    PrintRuleViolation(ruleName, severity, description, fileName, lineNumber, tentative);
 }
 
 void OutputPrinter::PrintRuleViolation(const std::string& ruleName,
                                        Severity severity,
                                        const std::string& description,
                                        const std::string& fileName,
-                                       int lineNumber)
+                                       int lineNumber,
+                                       bool tentative)
 {
     if (ShouldPrintLine(fileName, lineNumber))
     {
-        PrintRuleViolationImpl(ruleName, severity, description, fileName, lineNumber);
+        if (tentative)
+        {
+            m_tentativeViolations.emplace_back(ruleName, severity, description, fileName, lineNumber);
+        }
+        else
+        {
+            PrintRuleViolationImpl(ruleName, severity, description, fileName, lineNumber);
+        }
     }
 }
 
+void OutputPrinter::ClearTentativeViolations()
+{
+    m_tentativeViolations.clear();
+}
+
 bool OutputPrinter::ShouldPrintLine(const std::string& fileName, int lineNumber)
 {
     if (m_outputFilters.empty())
@@ -215,6 +253,22 @@ std::string OutputPrinter::GetSeverityString(Severity severity)
     return str;
 }
 
+void OutputPrinter::Save()
+{
+    for (const auto& violationInfo : m_tentativeViolations)
+    {
+        PrintRuleViolation(violationInfo.ruleName,
+                           violationInfo.severity,
+                           violationInfo.description,
+                           violationInfo.fileName,
+                           violationInfo.lineNumber);
+    }
+
+    m_tentativeViolations.clear();
+
+    SaveImpl();
+}
+
 ///////////////////////////
 
 PlainTextOutputPrinter::PlainTextOutputPrinter(const std::string& outputFileName,
@@ -247,7 +301,7 @@ void PlainTextOutputPrinter::PrintGraphEdge(const std::string& source,
     assert(false && "Not implemented");
 }
 
-void PlainTextOutputPrinter::Save()
+void PlainTextOutputPrinter::SaveImpl()
 {
 }
 
@@ -304,7 +358,7 @@ void XmlOutputPrinter::PrintGraphEdge(const std::string& source,
     assert(false && "Not implemented");
 }
 
-void XmlOutputPrinter::Save()
+void XmlOutputPrinter::SaveImpl()
 {
     m_resultsElement->LinkEndChild(m_errorsElement.release());
     m_document.LinkEndChild(m_resultsElement.release());
@@ -342,7 +396,7 @@ void DotGraphOutputPrinter::PrintGraphEdge(const std::string& source,
     m_graphEdges.insert(DotGraphEdge{source, destination, options});
 }
 
-void DotGraphOutputPrinter::Save()
+void DotGraphOutputPrinter::SaveImpl()
 {
     if (m_outputFileName.empty())
     {

diff --git a/Common/OutputPrinter.h b/Common/OutputPrinter.h
@@ -46,30 +46,37 @@ class OutputPrinter
                             const std::string& description,
                             clang::SourceLocation location,
                             clang::SourceManager& sourceManager,
-                            int lineOffset = 0);
+                            int lineOffset = 0,
+                            bool tentative = false);
 
     void PrintRuleViolation(const std::string& ruleName,
                             Severity severity,
                             const std::string& description,
                             const std::string& fileName,
-                            int lineNumber);
+                            int lineNumber,
+                            bool tentative = false);
 
     virtual void PrintGraphEdge(const std::string& source,
                                 const std::string& destination,
                                 const std::string& options = "") = 0;
 
-    virtual void Save() = 0;
+    void ClearTentativeViolations();
+
+    void Save();
 
 protected:
     virtual void PrintRuleViolationImpl(const std::string& ruleName,
                                         Severity severity,
                                         const std::string& description,
                                         const std::string& fileName,
                                         int lineNumber) = 0;
+    virtual void SaveImpl() = 0;
     bool ShouldPrintLine(const std::string& fileName, int lineNumber);
     std::string GetSeverityString(Severity severity);
 
 protected:
     const std::string m_outputFileName;
     const std::vector<OutputFilter> m_outputFilters;
+    struct RuleViolationInfo;
+    std::vector<RuleViolationInfo> m_tentativeViolations;
 };
diff --git a/Common/SourceFileInfo.h b/Common/SourceFileInfo.h
@@ -0,0 +1,9 @@
+#pragma once
+
+#include <string>
+
+struct SourceFileInfo
+{
+    std::string fileName;
+    int lineNumber = 0;
+};
diff --git a/Rules/RulesFactory.cpp b/Rules/RulesFactory.cpp
@@ -16,6 +16,7 @@
 #include "Rules/OldStyleNullPointerRule.h"
 #include "Rules/PossibleForwardDeclarationRule.h"
 #include "Rules/TodoRule.h"
+#include "Rules/UndefinedFunctionRule.h"
 #include "Rules/UninitializedFieldRule.h"
 #include "Rules/UninitializedLocalVariableRule.h"
 #include "Rules/UnusedForwardDeclarationRule.h"
@@ -73,5 +74,6 @@ std::vector<std::unique_ptr<Rule>> CreateRules(Context& context)
     AddRule<BlockPlacementRule>(rules, context);
     AddRule<WhitespaceRule>(rules, context);
     AddRule<LicenseInHeaderRule>(rules, context);
+    AddRule<UndefinedFunctionRule>(rules, context);
     return rules;
 }
diff --git a/Rules/UndefinedFunctionRule.cpp b/Rules/UndefinedFunctionRule.cpp
@@ -0,0 +1,96 @@
+#include "Rules/UndefinedFunctionRule.h"
+
+#include "Common/Context.h"
+#include "Common/FilenameHelper.h"
+#include "Common/OutputPrinter.h"
+#include "Common/SourceLocationHelper.h"
+
+#include <clang/AST/Decl.h>
+
+#include <boost/format.hpp>
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang
+{
+namespace ast_matchers
+{
+
+AST_MATCHER(FunctionDecl, isAnyTemplateKind)
+{
+    return Node.getTemplatedKind() != FunctionDecl::TK_NonTemplate;
+}
+
+AST_MATCHER(FunctionDecl, isDefaulted)
+{
+    return Node.isDefaulted();
+}
+
+} // namespace ast_matchers
+} // namespace clang
+
+
+UndefinedFunctionRule::UndefinedFunctionRule(Context& context)
+    : Rule(context)
+{}
+
+void UndefinedFunctionRule::RegisterASTMatcherCallback(clang::ast_matchers::MatchFinder& finder)
+{
+    finder.addMatcher(
+        functionDecl(unless(anyOf(isImplicit(),
+                                  isDefaulted(),
+                                  isDeleted(),
+                                  isAnyTemplateKind())),
+                     unless(methodDecl(isPure())))
+            .bind("functionDecl"),
+    this);
+}
+
+void UndefinedFunctionRule::run(const clang::ast_matchers::MatchFinder::MatchResult& result)
+{
+    m_sourceManager = result.SourceManager;
+
+    const FunctionDecl* functionDeclaration = result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
+    if (functionDeclaration == nullptr)
+        return;
+
+    SourceLocation location = functionDeclaration->getLocation();
+    if (! m_context.sourceLocationHelper.IsLocationOfInterest(GetName(), location, *m_sourceManager))
+        return;
+
+    std::string fullyQualifiedName = functionDeclaration->getQualifiedNameAsString();
+    if (functionDeclaration->isThisDeclarationADefinition())
+    {
+        m_context.definedFunctions.insert(fullyQualifiedName);
+        m_context.undefinedFunctions.erase(fullyQualifiedName);
+    }
+    else
+    {
+        if (m_context.definedFunctions.count(fullyQualifiedName) == 0)
+        {
+            SourceFileInfo info;
+            info.fileName = GetCleanFilename(location, *m_sourceManager);
+            info.lineNumber = m_sourceManager->getPresumedLineNumber(location);
+            m_context.undefinedFunctions.insert(std::make_pair(fullyQualifiedName, info));
+        }
+    }
+}
+
+void UndefinedFunctionRule::onEndOfTranslationUnit()
+{
+    m_context.outputPrinter->ClearTentativeViolations();
+
+    for (const auto& undefinedFunction : m_context.undefinedFunctions)
+    {
+        const bool tentative = true;
+        m_context.outputPrinter->PrintRuleViolation(
+            "undefined function",
+            Severity::Information,
+            boost::str(boost::format("Function '%s' declared but never defined")
+                % undefinedFunction.first),
+            undefinedFunction.second.fileName,
+            undefinedFunction.second.lineNumber,
+            tentative);
+    }
+}