Skip to content

Commit 56cd45a

Browse files
authored
bubble-up parsing problems via InheritedContext (#3218)
## Changes Refactoring ### Linked issues None ### Functionality None ### Tests - [X] ran unit tests --------- Co-authored-by: Eric Vergnaud <eric.vergnaud@databricks.com>
1 parent e52b1e9 commit 56cd45a

File tree

6 files changed

+53
-47
lines changed

6 files changed

+53
-47
lines changed

src/databricks/labs/ucx/source_code/graph.py

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -527,11 +527,16 @@ def failed(self) -> bool:
527527
return len(self.problems) > 0
528528

529529

530+
@dataclass
530531
class InheritedContext:
531532

533+
tree: Tree | None
534+
found: bool
535+
problems: Iterable[DependencyProblem]
536+
532537
@classmethod
533538
def from_route(cls, graph: DependencyGraph, path_lookup: PathLookup, route: list[Dependency]) -> InheritedContext:
534-
context = InheritedContext(None, False)
539+
context = InheritedContext(None, False, [])
535540
for i, dependency in enumerate(route):
536541
if i >= len(route) - 1:
537542
break
@@ -545,41 +550,29 @@ def from_route(cls, graph: DependencyGraph, path_lookup: PathLookup, route: list
545550
context = context.append(local, i == len(route) - 2)
546551
return context.finalize()
547552

548-
def __init__(self, tree: Tree | None, found: bool):
549-
self._tree = tree
550-
self._found = found
551-
552-
@property
553-
def tree(self) -> Tree | None:
554-
return self._tree
555-
556-
@property
557-
def found(self) -> bool:
558-
return self._found
559-
560553
def append(self, context: InheritedContext, copy_found: bool) -> InheritedContext:
561554
# we should never append to a found context
562555
if self.found:
563556
raise ValueError("Appending to an already resolved InheritedContext is illegal!")
564557
tree = context.tree
565558
found = copy_found and context.found
566559
if tree is None:
567-
return InheritedContext(self._tree, found)
568-
if self._tree is None:
569-
self._tree = Tree.new_module()
570-
self._tree.append_tree(tree)
571-
return InheritedContext(self._tree, found)
560+
return InheritedContext(self.tree, found, context.problems)
561+
new_tree = self.tree or Tree.new_module()
562+
new_tree.append_tree(tree)
563+
new_problems = itertools.chain(self.problems, context.problems)
564+
return InheritedContext(new_tree, found, new_problems)
572565

573566
def finalize(self) -> InheritedContext:
574567
# hacky stuff for aligning with Astroid's inference engine behavior
575568
# the engine checks line numbers to skip variables that are not in scope of the current frame
576569
# see https://github.com/pylint-dev/astroid/blob/5b665e7e760a7181625a24b3635e9fec7b174d87/astroid/filter_statements.py#L113
577570
# this is problematic when linting code fragments that refer to inherited code with unrelated line numbers
578571
# here we fool the engine by pretending that all nodes from context have negative line numbers
579-
if self._tree is None:
572+
if self.tree is None:
580573
return self
581-
tree = self._tree.renumber(-1)
582-
return InheritedContext(tree, self.found)
574+
tree = self.tree.renumber(-1)
575+
return InheritedContext(tree, self.found, [])
583576

584577

585578
T = TypeVar("T")

src/databricks/labs/ucx/source_code/linters/files.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,13 @@ def build_inherited_context(self, graph: DependencyGraph, child_path: Path) -> I
6666
if self._language is CellLanguage.PYTHON:
6767
context = graph.new_dependency_graph_context()
6868
analyzer = PythonCodeAnalyzer(context, self._original_code)
69-
return analyzer.build_inherited_context(child_path)
70-
return InheritedContext(None, False)
69+
inherited = analyzer.build_inherited_context(child_path)
70+
problems = list(inherited.problems)
71+
for idx, problem in enumerate(problems):
72+
if problem.is_path_missing():
73+
problems[idx] = dataclasses.replace(problem, source_path=self._path)
74+
return dataclasses.replace(inherited, problems=problems)
75+
return InheritedContext(None, False, [])
7176

7277
def __repr__(self):
7378
return f"<LocalFile {self._path}>"

src/databricks/labs/ucx/source_code/notebooks/cells.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def __repr__(self):
8484
return f"{self.language.name}: {self._original_code[:20]}"
8585

8686
def build_inherited_context(self, _graph: DependencyGraph, _child_path: Path) -> InheritedContext:
87-
return InheritedContext(None, False)
87+
return InheritedContext(None, False, [])
8888

8989

9090
class PythonCell(Cell):
@@ -437,21 +437,21 @@ def notebook_path(self) -> Path | None:
437437
def build_inherited_context(self, context: DependencyGraphContext, child_path: Path) -> InheritedContext:
438438
path = self.notebook_path
439439
if path is None:
440-
logger.warning("Missing notebook path in %run command")
441-
return InheritedContext(None, False)
440+
problem = DependencyProblem('missing-notebook-path', "Missing notebook path in %run command")
441+
return InheritedContext(None, False, [problem])
442442
maybe = context.resolver.resolve_notebook(context.path_lookup, path, inherit_context=True)
443443
if not maybe.dependency:
444-
logger.warning(f"Could not load notebook {path}")
445-
return InheritedContext(None, False)
444+
problem = DependencyProblem('missing-notebook', f"Could not locate notebook {path}")
445+
return InheritedContext(None, False, [problem])
446446
child_path = maybe.dependency.path
447447
absolute_path = context.path_lookup.resolve(path)
448448
absolute_child = context.path_lookup.resolve(child_path)
449449
if absolute_path == absolute_child:
450-
return InheritedContext(None, True)
450+
return InheritedContext(None, True, [])
451451
container = maybe.dependency.load(context.path_lookup)
452452
if not container:
453-
logger.warning(f"Could not load notebook {path}")
454-
return InheritedContext(None, False)
453+
problem = DependencyProblem('corrupt-notebook', f"Could not load notebook {path}")
454+
return InheritedContext(None, False, [problem])
455455
return container.build_inherited_context(context.parent, child_path)
456456

457457

src/databricks/labs/ucx/source_code/notebooks/magic.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def build_inherited_context(self, context: DependencyGraphContext, child_path: P
7373
magic = self.as_magic()
7474
if magic is not None:
7575
return magic.build_inherited_context(context, child_path)
76-
return InheritedContext(None, False)
76+
return InheritedContext(None, False, [])
7777

7878

7979
class MagicNode(NodeNG):
@@ -90,7 +90,7 @@ def __init__(self, node: NodeNG, code: str):
9090
def build_dependency_graph(self, parent: DependencyGraph) -> list[DependencyProblem]: ...
9191

9292
def build_inherited_context(self, _context: DependencyGraphContext, _child_path: Path) -> InheritedContext:
93-
return InheritedContext(None, False)
93+
return InheritedContext(None, False, [])
9494

9595

9696
_FACTORIES: list[Callable[[str, NodeNG], MagicCommand | None]] = []

src/databricks/labs/ucx/source_code/notebooks/sources.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,11 @@ def build_dependency_graph(self, parent: DependencyGraph) -> list[DependencyProb
110110
return problems
111111

112112
def build_inherited_context(self, graph: DependencyGraph, child_path: Path) -> InheritedContext:
113-
context = InheritedContext(None, False)
113+
problems: list[DependencyProblem] = []
114+
context = InheritedContext(None, False, problems)
114115
for cell in self._cells:
115-
child = cell.build_inherited_context(graph, child_path)
116-
context = context.append(child, True)
116+
child_context = cell.build_inherited_context(graph, child_path)
117+
context = context.append(child_context, True)
117118
if context.found:
118119
return context
119120
return context

src/databricks/labs/ucx/source_code/python/python_analyzer.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,11 @@ def build_graph(self) -> list[DependencyProblem]:
6262
def build_inherited_context(self, child_path: Path) -> InheritedContext:
6363
tree, nodes, problems = self._parse_and_extract_nodes()
6464
if problems:
65-
# TODO: bubble up problems via InheritedContext
66-
logger.warning(f"Failed to parse: {problems}")
67-
return InheritedContext(None, False)
65+
return InheritedContext(None, False, problems)
6866
assert tree is not None, "no problems should yield a tree"
6967
if len(nodes) == 0:
70-
return InheritedContext(tree, False)
71-
context = InheritedContext(Tree.new_module(), False)
68+
return InheritedContext(tree, False, [])
69+
context = InheritedContext(Tree.new_module(), False, [])
7270
assert context.tree is not None, "Tree should be initialized"
7371
last_line = -1
7472
for base_node in nodes:
@@ -123,22 +121,31 @@ def _build_graph_from_node(self, base_node: NodeBase) -> Iterable[DependencyProb
123121
elif isinstance(base_node, MagicLine):
124122
yield from base_node.build_dependency_graph(self._context.parent)
125123
else:
126-
logger.warning(f"Can't build graph for node {NodeBase.__name__} of type {type(base_node).__name__}")
124+
problem = DependencyProblem.from_node(
125+
"unsupported-node-type",
126+
f"Can't build graph for node {NodeBase.__name__} of type {type(base_node).__name__}",
127+
base_node.node,
128+
)
129+
yield problem
127130

128131
def _build_inherited_context_from_node(self, base_node: NodeBase, child_path: Path) -> InheritedContext:
129132
if isinstance(base_node, SysPathChange):
130133
self._mutate_path_lookup(base_node)
131-
return InheritedContext(None, False)
134+
return InheritedContext(None, False, [])
132135
if isinstance(base_node, ImportSource):
133136
# nothing to do, Astroid takes care of imports
134-
return InheritedContext(None, False)
137+
return InheritedContext(None, False, [])
135138
if isinstance(base_node, NotebookRunCall):
136139
# nothing to do, dbutils.notebook.run uses a dedicated context
137-
return InheritedContext(None, False)
140+
return InheritedContext(None, False, [])
138141
if isinstance(base_node, MagicLine):
139142
return base_node.build_inherited_context(self._context, child_path)
140-
logger.warning(f"Can't build inherited context for node {NodeBase.__name__} of type {type(base_node).__name__}")
141-
return InheritedContext(None, False)
143+
problem = DependencyProblem.from_node(
144+
"unsupported-node-type",
145+
f"Can't build inherited context for node {NodeBase.__name__} of type {type(base_node).__name__}",
146+
base_node.node,
147+
)
148+
return InheritedContext(None, False, [problem])
142149

143150
def _register_import(self, base_node: ImportSource) -> Iterable[DependencyProblem]:
144151
prefix = ""

0 commit comments

Comments
 (0)