Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) Fix bug with renaming when a formula uses a local name for a user table.

Browse Source 
Summary:
When a formula used a local variable referring to a user table (which is
a global name), e.g. `myvar = MyTable; myvar.lookupOne(...)`, renaming
logic mistakenly used the inferred name (`MyTable`) in places where the
actual variable name (`myvar`) should have been used.

Additionally, we should not rename local variables, even if they match a
global name.

This fixes both issues.

Test Plan: Added a test case that checks that local variables aren't renamed.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3846
This commit is contained in:
Dmitry S 2023-03-30 23:51:13 -04:00
parent d8a063284a
commit fb1332d529
2 changed files with 33 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
sandbox/grist/codebuilder.py
View File

@ -195,6 +195,11 @@ def _is_table(node):
return (isinstance(node, astroid.nodes.ClassDef) and node.decorators and
node.decorators.nodes[0].as_string() == 'grist.UserTable')
def _is_local(node):
"""
Returns true if node is a Name node for an innermost variable.
"""
return isinstance(node, astroid.nodes.Name) and node.name in node.scope().locals
@contextlib.contextmanager
@ -394,14 +399,15 @@ def parse_grist_names(builder):
in_text, in_value, in_patch = patch_source
if in_value:
return (in_value, in_patch.start, table_id, col_id)
return None
parsed_names = []
for node in asttokens.util.walk(atok.tree):
if isinstance(node, astroid.nodes.Name):
obj = infer(node)
if _is_table(obj):
if _is_table(obj) and not _is_local(node):
start, end = atok.get_text_range(node)
parsed_names.append(make_tuple(start, end, obj.name, None))
parsed_names.append(make_tuple(start, end, node.name, None))
elif isinstance(node, astroid.nodes.Attribute):
obj = infer(node.expr)
@ -431,7 +437,7 @@ def save_to_linecache(source_code):
Makes source code available to friendly-traceback and traceback formatting in general.
"""
if six.PY3:
import friendly_traceback.source_cache
import friendly_traceback.source_cache # pylint: disable=import-error
friendly_traceback.source_cache.cache.add(code_filename, source_code)
else:

24
sandbox/grist/test_renames2.py
View File

@ -395,3 +395,27 @@ class TestRenames2(test_engine.EngineTestCase):
self.assertTableData("People", cols="subset", data=self.people_data)
self.assertTableData("Games", cols="subset", data=self.games_data)
def test_renames_i(self):
# Rename when using a local variable referring to a user table.
# Test also that a local variable that happens to match a global name is unaffected by renames.
self.modify_column("Games", "winner", formula=(
"myvar = Entries\n"
"People = Entries\n"
"myvar.lookupOne(game=$id, rank=1).person\n"
"People.lookupOne(game=$id, rank=1).person\n"
))
self.apply_user_action(["RenameColumn", "Entries", "game", "juego"])
self.apply_user_action(["RenameTable", "People", "Persons"])
# Check that renames worked.
new_col = self.engine.docmodel.columns.lookupOne(tableId='Games', colId='winner')
self.assertEqual(new_col.formula, (
"myvar = Entries\n"
"People = Entries\n"
"myvar.lookupOne(juego=$id, rank=1).person\n"
"People.lookupOne(juego=$id, rank=1).person\n"
))
self.assertTableData("Persons", cols="subset", data=self.people_data)
self.assertTableData("Games", cols="subset", data=self.games_data)