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

Suggest alternatives when identifier not found. Closes #3058. #3147

Merged
merged 8 commits into from Feb 13, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Expand Up @@ -8,6 +8,7 @@ Features:
* Inline Assembly: Issue warning for using jump labels (already existed for jump instructions).
* Inline Assembly: Support some restricted tokens (return, byte, address) as identifiers in Julia mode.
* Optimiser: Replace `x % 2**i` by `x & (2**i-1)`.
* Resolver: Suggest alternative identifiers if a given identifier is not found.
* SMT Checker: If-else branch conditions are taken into account in the SMT encoding of the program
variables.
* Syntax Checker: Deprecate the ``var`` keyword (and mark it an error as experimental 0.5.0 feature).
Expand Down
101 changes: 101 additions & 0 deletions libdevcore/StringUtils.cpp
@@ -0,0 +1,101 @@
/*
This file is part of solidity.

solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
/** @file StringUtils.h
* @author Balajiganapathi S <balajiganapathi.s@gmail.com>
* @date 2017
*
* String routines
*/

#include "StringUtils.h"
#include <algorithm>
#include <string>
#include <vector>

using namespace std;
using namespace dev;

bool dev::stringWithinDistance(string const& _str1, string const& _str2, size_t _maxDistance)
{
if (_str1 == _str2)
return true;

size_t n1 = _str1.size();
size_t n2 = _str2.size();
size_t distance = stringDistance(_str1, _str2);

// if distance is not greater than _maxDistance, and distance is strictly less than length of both names, they can be considered similar
// this is to avoid irrelevant suggestions
return distance <= _maxDistance && distance < n1 && distance < n2;
}

size_t dev::stringDistance(string const& _str1, string const& _str2)
{
size_t n1 = _str1.size();
size_t n2 = _str2.size();
// Optimize by storing only last 2 rows and current row. So first index is considered modulo 3
// This is a two-dimensional array of size 3 x (n2 + 1).
vector<size_t> dp(3 * (n2 + 1));

// In this dp formulation of Damerau–Levenshtein distance we are assuming that the strings are 1-based to make base case storage easier.
// So index accesser to _name1 and _name2 have to be adjusted accordingly
for (size_t i1 = 0; i1 <= n1; ++i1)
for (size_t i2 = 0; i2 <= n2; ++i2)
{
size_t x = 0;
if (min(i1, i2) == 0) // base case
x = max(i1, i2);
else
{
size_t left = dp[(i1 - 1) % 3 + i2 * 3];
size_t up = dp[(i1 % 3) + (i2 - 1) * 3];
size_t upleft = dp[((i1 - 1) % 3) + (i2 - 1) * 3];
// deletion and insertion
x = min(left + 1, up + 1);
if (_str1[i1-1] == _str2[i2-1])
// same chars, can skip
x = min(x, upleft);
else
// different chars so try substitution
x = min(x, upleft + 1);

// transposing
if (i1 > 1 && i2 > 1 && _str1[i1 - 1] == _str2[i2 - 2] && _str1[i1 - 2] == _str2[i2 - 1])
x = min(x, dp[((i1 - 2) % 3) + (i2 - 2) * 3] + 1);
}
dp[(i1 % 3) + i2 * 3] = x;
}

return dp[(n1 % 3) + n2 * 3];
}

string dev::quotedAlternativesList(vector<string> const& suggestions)
{
if (suggestions.empty())
return "";
if (suggestions.size() == 1)
return "\"" + suggestions.front() + "\"";

string choices = "\"" + suggestions.front() + "\"";
for (size_t i = 1; i + 1 < suggestions.size(); ++i)
choices += ", \"" + suggestions[i] + "\"";

choices += " or \"" + suggestions.back() + "\"";

return choices;
}

39 changes: 39 additions & 0 deletions libdevcore/StringUtils.h
@@ -0,0 +1,39 @@
/*
This file is part of solidity.

solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
/** @file StringUtils.h
* @author Balajiganapathi S <balajiganapathi.s@gmail.com>
* @date 2017
*
* String routines
*/

#pragma once

#include <string>
#include <vector>

namespace dev
{

// Calculates the Damerau–Levenshtein distance between _str1 and _str2 and returns true if that distance is not greater than _maxDistance
bool stringWithinDistance(std::string const& _str1, std::string const& _str2, size_t _maxDistance);
// Calculates the Damerau–Levenshtein distance between _str1 and _str2
size_t stringDistance(std::string const& _str1, std::string const& _str2);
// Return a string having elements of suggestions as quoted, alternative suggestions. e.g. "a", "b" or "c"
std::string quotedAlternativesList(std::vector<std::string> const& suggestions);

}
22 changes: 21 additions & 1 deletion libsolidity/analysis/DeclarationContainer.cpp
Expand Up @@ -23,6 +23,7 @@
#include <libsolidity/analysis/DeclarationContainer.h>
#include <libsolidity/ast/AST.h>
#include <libsolidity/ast/Types.h>
#include <libdevcore/StringUtils.h>

using namespace std;
using namespace dev;
Expand Down Expand Up @@ -105,7 +106,7 @@ bool DeclarationContainer::registerDeclaration(
return true;
}

std::vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _name, bool _recursive) const
vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _name, bool _recursive) const
{
solAssert(!_name.empty(), "Attempt to resolve empty name.");
auto result = m_declarations.find(_name);
Expand All @@ -115,3 +116,22 @@ std::vector<Declaration const*> DeclarationContainer::resolveName(ASTString cons
return m_enclosingContainer->resolveName(_name, true);
return vector<Declaration const*>({});
}

vector<ASTString> DeclarationContainer::similarNames(ASTString const& _name) const
{
static size_t const MAXIMUM_EDIT_DISTANCE = 2;

vector<ASTString> similar;

for (auto const& declaration: m_declarations)
{
string const& declarationName = declaration.first;
if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE))
similar.push_back(declarationName);
}

if (m_enclosingContainer)
similar += m_enclosingContainer->similarNames(_name);

return similar;
}
4 changes: 4 additions & 0 deletions libsolidity/analysis/DeclarationContainer.h
Expand Up @@ -58,6 +58,10 @@ class DeclarationContainer
/// @returns whether declaration is valid, and if not also returns previous declaration.
Declaration const* conflictingDeclaration(Declaration const& _declaration, ASTString const* _name = nullptr) const;

/// @returns existing declaration names similar to @a _name.
/// Searches this and all parent containers.
std::vector<ASTString> similarNames(ASTString const& _name) const;

private:
ASTNode const* m_enclosingNode;
DeclarationContainer const* m_enclosingContainer;
Expand Down
6 changes: 6 additions & 0 deletions libsolidity/analysis/NameAndTypeResolver.cpp
Expand Up @@ -25,6 +25,7 @@
#include <libsolidity/ast/AST.h>
#include <libsolidity/analysis/TypeChecker.h>
#include <libsolidity/interface/ErrorReporter.h>
#include <libdevcore/StringUtils.h>

#include <boost/algorithm/string.hpp>

Expand Down Expand Up @@ -425,6 +426,11 @@ vector<_T const*> NameAndTypeResolver::cThreeMerge(list<list<_T const*>>& _toMer
return result;
}

string NameAndTypeResolver::similarNameSuggestions(ASTString const& _name) const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a function in StringUtils called something like quotedAlternativesList.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

{
return quotedAlternativesList(m_currentScope->similarNames(_name));
}

DeclarationRegistrationHelper::DeclarationRegistrationHelper(
map<ASTNode const*, shared_ptr<DeclarationContainer>>& _scopes,
ASTNode& _astRoot,
Expand Down
3 changes: 3 additions & 0 deletions libsolidity/analysis/NameAndTypeResolver.h
Expand Up @@ -93,6 +93,9 @@ class NameAndTypeResolver: private boost::noncopyable
/// Generate and store warnings about variables that are named like instructions.
void warnVariablesNamedLikeInstructions();

/// @returns a list of similar identifiers in the current and enclosing scopes. May return empty string if no suggestions.
std::string similarNameSuggestions(ASTString const& _name) const;

private:
/// Internal version of @a resolveNamesAndTypes (called from there) throws exceptions on fatal errors.
bool resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode = true);
Expand Down
8 changes: 7 additions & 1 deletion libsolidity/analysis/ReferencesResolver.cpp
Expand Up @@ -47,7 +47,13 @@ bool ReferencesResolver::visit(Identifier const& _identifier)
{
auto declarations = m_resolver.nameFromCurrentScope(_identifier.name());
if (declarations.empty())
declarationError(_identifier.location(), "Undeclared identifier.");
{
string suggestions = m_resolver.similarNameSuggestions(_identifier.name());
string errorMessage =
"Undeclared identifier." +
(suggestions.empty()? "": " Did you mean " + std::move(suggestions) + "?");
declarationError(_identifier.location(), errorMessage);
}
else if (declarations.size() == 1)
_identifier.annotation().referencedDeclaration = declarations.front();
else
Expand Down
88 changes: 88 additions & 0 deletions test/libdevcore/StringUtils.cpp
@@ -0,0 +1,88 @@
/*
This file is part of solidity.

solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
/**
* Unit tests for the StringUtils routines.
*/

#include <libdevcore/StringUtils.h>

#include "../TestHelper.h"

using namespace std;

namespace dev
{
namespace test
{

BOOST_AUTO_TEST_SUITE(StringUtils)

BOOST_AUTO_TEST_CASE(test_similarity)
{
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "hello", 0), true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tests here would also profit greatly from a separate distance function.

BOOST_CHECK_EQUAL(stringWithinDistance("hello", "hello", 1), true);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "hellw", 1), true);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "helol", 1), true);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "helo", 1), true);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "helllo", 1), true);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "hlllo", 1), true);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "hllllo", 1), false);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "hllllo", 2), true);
BOOST_CHECK_EQUAL(stringWithinDistance("hello", "hlllo", 2), true);
BOOST_CHECK_EQUAL(stringWithinDistance("a", "", 2), false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be distance 1?

BOOST_CHECK_EQUAL(stringWithinDistance("abc", "ba", 2), false);
BOOST_CHECK_EQUAL(stringWithinDistance("abc", "abcdef", 2), false);
BOOST_CHECK_EQUAL(stringWithinDistance("abcd", "wxyz", 2), false);
BOOST_CHECK_EQUAL(stringWithinDistance("", "", 2), true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be distance 0?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method checks if it is distance <= n: 0 <= 2.

}

BOOST_AUTO_TEST_CASE(test_dldistance)
{
BOOST_CHECK_EQUAL(stringDistance("hello", "hellw"), 1);
BOOST_CHECK_EQUAL(stringDistance("hello", "helol"), 1);
BOOST_CHECK_EQUAL(stringDistance("hello", "helo"), 1);
BOOST_CHECK_EQUAL(stringDistance("hello", "helllo"), 1);
BOOST_CHECK_EQUAL(stringDistance("hello", "hlllo"), 1);
BOOST_CHECK_EQUAL(stringDistance("hello", "hllllo"), 2);
BOOST_CHECK_EQUAL(stringDistance("a", ""), 1);
BOOST_CHECK_EQUAL(stringDistance("abc", "ba"), 2);
BOOST_CHECK_EQUAL(stringDistance("abc", "abcdef"), 3);
BOOST_CHECK_EQUAL(stringDistance("abcd", "wxyz"), 4);
BOOST_CHECK_EQUAL(stringDistance("", ""), 0);
BOOST_CHECK_EQUAL(stringDistance("abcdefghijklmnopqrstuvwxyz", "abcabcabcabcabcabcabcabca"), 23);

}

BOOST_AUTO_TEST_CASE(test_alternatives_list)
{
vector<string> strings;
BOOST_CHECK_EQUAL(quotedAlternativesList(strings), "");
strings.push_back("a");
BOOST_CHECK_EQUAL(quotedAlternativesList(strings), "\"a\"");
strings.push_back("b");
BOOST_CHECK_EQUAL(quotedAlternativesList(strings), "\"a\" or \"b\"");
strings.push_back("c");
BOOST_CHECK_EQUAL(quotedAlternativesList(strings), "\"a\", \"b\" or \"c\"");
strings.push_back("d");
BOOST_CHECK_EQUAL(quotedAlternativesList(strings), "\"a\", \"b\", \"c\" or \"d\"");
}


BOOST_AUTO_TEST_SUITE_END()

}
}