search for variable in parent scopes when checking for shadowing

rstudio · Apr 23, 2015 · 55239a4 · 55239a4
1 parent 4738594
commit 55239a4
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 13 deletions.
diff --git a/src/cpp/session/modules/SessionRParser.cpp b/src/cpp/session/modules/SessionRParser.cpp
@@ -974,7 +974,7 @@ FunctionInformation getInfoAssociatedWithFunctionAtCursor(
       // 'pf' is now actually bound to; this could be doable in some simple cases
       // but the pattern is uncommon enough that it's better that we just don't
       // supply incorrect diagnostics, rather than attempt to supply correct diagnostics.
-      if (status.node()->getDefinedSymbols().count(cursor.contentAsUtf8()))
+      if (status.node()->findVariable(cursor.contentAsUtf8(), cursor.currentPosition()))
          return FunctionInformation();
 
       // If we're within a package project, then attempt searching the

diff --git a/src/cpp/session/modules/SessionRParser.hpp b/src/cpp/session/modules/SessionRParser.hpp
@@ -30,13 +30,15 @@
 #include <iostream>
 #include <iomanip>
 
+#include <core/Algorithm.hpp>
 #include <core/r_util/RTokenizer.hpp>
 #include <core/collection/Position.hpp>
 #include <core/collection/Stack.hpp>
 
 #include <r/RSexp.hpp>
 #include <r/RExec.hpp>
 
+#include <boost/bind.hpp>
 #include <boost/container/flat_set.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/foreach.hpp>
@@ -631,16 +633,18 @@ class ParseNode : public boost::noncopyable
       return searchPathObjects.count(symbol) != 0;
    }
 
-   static bool doFindFunction(const ParseNode* pNode,
-                              const std::string& name,
-                              const Position& position,
-                              const ParseNode** ppFoundNode)
+private:
+
+   static bool findFunctionImpl(const ParseNode* pNode,
+                                const std::string& name,
+                                const Position& position,
+                                const ParseNode** ppFoundNode)
    {
       if (!pNode) return false;
 
       if (pNode->name_ == name && pNode->position_ <= position)
       {
-         *ppFoundNode = pNode;
+         if (ppFoundNode) *ppFoundNode = pNode;
          return true;
       }
 
@@ -654,28 +658,92 @@ class ParseNode : public boost::noncopyable
          const boost::shared_ptr<ParseNode>& pChild = pNode->children_[index];
          if (pChild->name_ == name && pChild->position_ <= position)
          {
-            *ppFoundNode = pChild.get();
+            if (ppFoundNode) *ppFoundNode = pChild.get();
             return true;
          }
       }
 
-      return doFindFunction(pNode->pParent_,
-                            name,
-                            position,
-                            ppFoundNode);
+      return findFunctionImpl(
+               pNode->pParent_,
+               name,
+               position,
+               ppFoundNode);
    }
 
+public:
+
    bool findFunction(const std::string& name,
                      const Position& position,
-                     const ParseNode** ppFoundNode) const
+                     const ParseNode** ppFoundNode = NULL) const
    {
-      return doFindFunction(
+      return findFunctionImpl(
                this,
                name,
                position,
                ppFoundNode);
    }
 
+private:
+
+   static bool findVariableImpl(const ParseNode* pNode,
+                                const std::string& name,
+                                const Position& position,
+                                bool checkPosition,
+                                Position* pFoundPosition)
+   {
+      if (!pNode) return false;
+
+      // First, perform a position-wide search in the current node.
+      Positions* pPositions = NULL;
+      core::algorithm::get(pNode->getDefinedSymbols(), name, &pPositions);
+
+      if (!pPositions)
+         return findVariableImpl(pNode->getParent(),
+                                 name,
+                                 position,
+                                 false,
+                                 pFoundPosition);
+
+      std::size_t n = pPositions->size();
+      if (checkPosition)
+      {
+         for (std::size_t i = n; i != 0; --i)
+         {
+            Position& definitionPos = (*pPositions)[i - 1];
+            if (definitionPos < position)
+            {
+               if (pFoundPosition) *pFoundPosition = definitionPos;
+               return true;
+            }
+         }
+      }
+      else if (n > 0)
+      {
+         if (pFoundPosition) *pFoundPosition = (*pPositions)[n - 1];
+         return true;
+      }
+
+      return findVariableImpl(
+               pNode->getParent(),
+               name,
+               position,
+               false,
+               pFoundPosition);
+   }
+
+public:
+
+   bool findVariable(const std::string& name,
+                     const Position& position,
+                     Position* pFoundPosition = NULL) const
+   {
+      return findVariableImpl(this,
+                              name,
+                              position,
+                              true,
+                              pFoundPosition);
+   }
+
    bool symbolHasDefinitionInTree(const std::string& symbol,
                                   const Position& position) const
    {