Skip to content

Commit

Permalink
UnusedImport linter: detect use of imported struct typedefs
Browse files Browse the repository at this point in the history
rename 'called' markers to 'used
  • Loading branch information
mlin committed Sep 11, 2019
1 parent 1f0016f commit 87d5fc7
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 25 deletions.
6 changes: 3 additions & 3 deletions WDL/CLI.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def descend(dobj=None, first_descent=first_descent):
if obj.workflow:
descend(obj.workflow)
# tasks
for task in sorted(obj.tasks, key=lambda task: (not task.called, task.name)):
for task in sorted(obj.tasks, key=lambda task: (not task.used, task.name)):
descend(task)
# imports
for imp in sorted(obj.imports, key=lambda t: t.namespace):
Expand All @@ -185,7 +185,7 @@ def descend(dobj=None, first_descent=first_descent):
elif isinstance(obj, Workflow):
print(
"{}workflow {}{}".format(
s, obj.name, " (not called)" if show_called and not obj.called else ""
s, obj.name, " (not called)" if show_called and not obj.used else ""
),
file=file,
)
Expand All @@ -195,7 +195,7 @@ def descend(dobj=None, first_descent=first_descent):
elif isinstance(obj, Task):
print(
"{}task {}{}".format(
s, obj.name, " (not called)" if show_called and not obj.called else ""
s, obj.name, " (not called)" if show_called and not obj.used else ""
),
file=file,
)
Expand Down
22 changes: 14 additions & 8 deletions WDL/Lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def lint(doc, descend_imports: bool = True):

# Add additional markups to the AST for use by the linters
Walker.SetParents()(doc)
Walker.MarkCalled()(doc)
Walker.MarkUsed()(doc)
Walker.SetReferrers()(doc)

# instantiate linters
Expand Down Expand Up @@ -605,15 +605,21 @@ class UnusedImport(Linter):
def document(self, obj: Tree.Document) -> Any:
for imp in obj.imports:
assert imp.doc is not None
any_called = False
any_used = False
for stb in obj.struct_typedefs:
st: Tree.StructTypeDef = stb.value
if st.imported and st.imported[0] is imp and getattr(st, "used", False):
any_used = True
for task in imp.doc.tasks:
if getattr(task, "called", False):
any_called = True
if imp.doc.workflow and getattr(imp.doc.workflow, "called", False):
any_called = True
if not any_called and (imp.doc.tasks or imp.doc.workflow):
if getattr(task, "used", False):
any_used = True
if imp.doc.workflow and getattr(imp.doc.workflow, "used", False):
any_used = True
if not any_used and (imp.doc.tasks or imp.doc.workflow):
self.add(
obj, "no calls to tasks/workflow in the imported document " + imp.namespace
obj,
"no calls to tasks/workflow, nor use of structs defined, in the imported document "
+ imp.namespace,
)


Expand Down
53 changes: 43 additions & 10 deletions WDL/Walker.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# pylint: disable=assignment-from-no-return
from typing import Any, List, Optional
from . import Error, Expr, Tree
from . import Error, Expr, Tree, Type


class Base:
Expand Down Expand Up @@ -236,22 +236,36 @@ def expr(self, obj: Expr.Base) -> None:
obj.parent = self._parent_stack[-1]


class MarkCalled(Base):
class MarkUsed(Base):
"""
Mark each Task and Workflow with ``called : bool`` according to whether
there exists a Call to it in the top-level workflow (or a subworkflow it
calls). Requires SetParents to have been applied previously.
Mark each Task and Workflow with ``used : bool`` according to whether there exists a Call to it
in the top-level workflow (or a called sub-workflow). The top-level workflow is considered
called.
The top-level workflow is considered called.
Also mark each StructTypeDef used if there exists a Decl instance of it in any workflow,
sub-workflow, or task (regardless of whether those are called). If the StructTypeDef is imported
from another document, propagate the flag there as well.
Requires SetParents to have been applied previously.
"""

marking: bool = False # True while recursing from the top-level workflow

def document(self, obj: Tree.Document) -> None:
for stb in obj.struct_typedefs:
st: Tree.StructTypeDef = stb.value
st.used = False
# if struct has members that are imported structs, mark those used
for ty in st.members.values():
self._mark_structs(obj, ty, imported_only=True)

super().document(obj)

def workflow(self, obj: Tree.Workflow) -> None:
obj.called = False
obj.used = False
if obj.parent.parent is None: # pyre-ignore
assert not self.marking
obj.called = True
obj.used = True
self.marking = True
super().workflow(obj)
self.marking = False
Expand All @@ -262,10 +276,29 @@ def call(self, obj: Tree.Call) -> None:
assert self.marking
if isinstance(obj.callee, Tree.Workflow):
self(obj.callee)
obj.callee.called = True
obj.callee.used = True

def task(self, obj: Tree.Task) -> None:
obj.called = False
obj.used = False
super().task(obj)

def decl(self, obj: Tree.Decl) -> None:
doc = obj
while not isinstance(doc, Tree.Document):
doc = getattr(doc, "parent")
self._mark_structs(doc, obj.type)

def _mark_structs(self, doc: Tree.Document, ty: Type.Base, imported_only: bool = False) -> None:
if isinstance(ty, Type.StructInstance):
st: Tree.StructTypeDef = doc.struct_typedefs[ty.type_name]
if not imported_only:
st.used = True
while st.imported:
st.used = True
st = st.imported[1]
st.used = True
for ch in ty.parameters:
self._mark_structs(doc, ch, imported_only=imported_only)


class SetReferrers(Base):
Expand Down
8 changes: 4 additions & 4 deletions tests/test_3corpi.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,30 +268,30 @@ class Contrived2(unittest.TestCase):
# these use the pattern 'input { Type? x = default }' and need check_quant=False
"mergecounts","somaticseq"
],
expected_lint={'UnusedImport': 8, 'OptionalCoercion': 9, 'StringCoercion': 14, 'UnusedDeclaration': 18, 'UnnecessaryQuantifier': 41, 'NonemptyCoercion': 1, 'NameCollision': 1, 'SelectArray': 1},
expected_lint={'OptionalCoercion': 9, 'StringCoercion': 14, 'UnusedDeclaration': 18, 'UnnecessaryQuantifier': 41, 'NonemptyCoercion': 1, 'NameCollision': 1, 'SelectArray': 1},
)
class BioWDLTasks(unittest.TestCase):
pass

@wdl_corpus(
["test_corpi/biowdl/aligning/**"],
expected_lint={'UnusedImport': 12, 'OptionalCoercion': 12, 'StringCoercion': 14, 'UnusedDeclaration': 12, 'UnnecessaryQuantifier': 41, 'NonemptyCoercion': 1, 'NameCollision': 1},
expected_lint={'OptionalCoercion': 12, 'StringCoercion': 14, 'UnusedDeclaration': 12, 'UnnecessaryQuantifier': 41, 'NonemptyCoercion': 1, 'NameCollision': 1},
check_quant=False,
)
class BioWDLAligning(unittest.TestCase):
pass

@wdl_corpus(
["test_corpi/biowdl/expression-quantification/**"],
expected_lint={'UnusedImport': 12, 'OptionalCoercion': 11, 'StringCoercion': 14, 'UnusedDeclaration': 12, 'UnnecessaryQuantifier': 41, 'NonemptyCoercion': 3, 'NameCollision': 1},
expected_lint={'OptionalCoercion': 11, 'StringCoercion': 14, 'UnusedDeclaration': 12, 'UnnecessaryQuantifier': 41, 'NonemptyCoercion': 3, 'NameCollision': 1},
check_quant=False,
)
class BioWDLExpressionQuantification(unittest.TestCase):
pass

@wdl_corpus(
["test_corpi/biowdl/somatic-variantcalling"],
expected_lint={'UnusedImport': 12, 'OptionalCoercion': 11, 'StringCoercion': 16, 'UnusedDeclaration': 11, 'UnnecessaryQuantifier': 166, 'NonemptyCoercion': 37, 'SelectArray': 5},
expected_lint={'UnusedImport': 2, 'OptionalCoercion': 11, 'StringCoercion': 16, 'UnusedDeclaration': 11, 'UnnecessaryQuantifier': 166, 'NonemptyCoercion': 37, 'SelectArray': 5},
check_quant=False,
)
class BioWDLSomaticVariantCalling(unittest.TestCase):
Expand Down

0 comments on commit 87d5fc7

Please sign in to comment.