diff options
author | Todd Gamblin <tgamblin@llnl.gov> | 2021-12-23 00:41:45 -0800 |
---|---|---|
committer | Greg Becker <becker33@llnl.gov> | 2022-01-12 06:14:18 -0800 |
commit | 8880a00862026700a82de1004e24ff06e54a165a (patch) | |
tree | 84d021c13ce3d3780284e455f1f7a366679285c0 /lib | |
parent | 572fbf4f49564669d8157567c6d18684cc6e086a (diff) | |
download | spack-8880a00862026700a82de1004e24ff06e54a165a.tar.gz spack-8880a00862026700a82de1004e24ff06e54a165a.tar.bz2 spack-8880a00862026700a82de1004e24ff06e54a165a.tar.xz spack-8880a00862026700a82de1004e24ff06e54a165a.zip |
package_hash: remove all unassigned strings, not just docstrings
Some packages use top-level unassigned strings instead of comments, either just after a
docstring on in the body somewhere else. Ignore those strings becasue they have no
effect on package behavior.
- [x] adjust RemoveDocstrings to remove all free-standing strings.
- [x] move tests for util/package_hash.py to test/util/package_hash.py
Co-authored-by: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/test/util/package_hash.py (renamed from lib/spack/spack/test/package_hash.py) | 70 | ||||
-rw-r--r-- | lib/spack/spack/util/package_hash.py | 16 |
2 files changed, 77 insertions, 9 deletions
diff --git a/lib/spack/spack/test/package_hash.py b/lib/spack/spack/test/util/package_hash.py index 6e50fee5ce..16829586e4 100644 --- a/lib/spack/spack/test/package_hash.py +++ b/lib/spack/spack/test/util/package_hash.py @@ -3,18 +3,22 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import ast + +import spack.paths +import spack.util.package_hash as ph from spack.spec import Spec -from spack.util.package_hash import package_content, package_hash +from spack.util.unparse import unparse def test_hash(tmpdir, mock_packages, config): - package_hash("hash-test1@1.2") + ph.package_hash("hash-test1@1.2") def test_different_variants(tmpdir, mock_packages, config): spec1 = Spec("hash-test1@1.2 +variantx") spec2 = Spec("hash-test1@1.2 +varianty") - assert package_hash(spec1) == package_hash(spec2) + assert ph.package_hash(spec1) == ph.package_hash(spec2) def test_all_same_but_name(tmpdir, mock_packages, config): @@ -55,11 +59,67 @@ def test_all_same_but_install(tmpdir, mock_packages, config): def compare_sans_name(eq, spec1, spec2): - content1 = package_content(spec1) + content1 = ph.package_content(spec1) content1 = content1.replace(spec1.package.__class__.__name__, '') - content2 = package_content(spec2) + content2 = ph.package_content(spec2) content2 = content2.replace(spec2.package.__class__.__name__, '') if eq: assert content1 == content2 else: assert content1 != content2 + + +many_strings = '''\ +"""ONE""" +"""TWO""" + +var = "THREE" # make sure this is not removed + +"FOUR" + +class ManyDocstrings: + """FIVE""" + """SIX""" + + x = "SEVEN" + + def method1(): + """EIGHT""" + + print("NINE") + + "TEN" + for i in range(10): + print(i) + + def method2(): + """ELEVEN""" + return "TWELVE" +''' + + +def test_remove_docstrings(): + tree = ast.parse(many_strings) + tree = ph.RemoveDocstrings().visit(tree) + + unparsed = unparse(tree) + + # make sure the methods are preserved + assert "method1" in unparsed + assert "method2" in unparsed + + # all of these are unassigned and should be removed + assert "ONE" not in unparsed + assert "TWO" not in unparsed + assert "FOUR" not in unparsed + assert "FIVE" not in unparsed + assert "SIX" not in unparsed + assert "EIGHT" not in unparsed + assert "TEN" not in unparsed + assert "ELEVEN" not in unparsed + + # these are used in legitimate expressions + assert "THREE" in unparsed + assert "SEVEN" in unparsed + assert "NINE" in unparsed + assert "TWELVE" in unparsed diff --git a/lib/spack/spack/util/package_hash.py b/lib/spack/spack/util/package_hash.py index bb9ad40e45..d1412e16fd 100644 --- a/lib/spack/spack/util/package_hash.py +++ b/lib/spack/spack/util/package_hash.py @@ -15,12 +15,20 @@ import spack.util.naming class RemoveDocstrings(ast.NodeTransformer): - """Transformer that removes docstrings from a Python AST.""" + """Transformer that removes docstrings from a Python AST. + + This removes *all* strings that aren't on the RHS of an assignment statement from + the body of functions, classes, and modules -- even if they're not directly after + the declaration. + + """ def remove_docstring(self, node): + def unused_string(node): + """Criteria for unassigned body strings.""" + return isinstance(node, ast.Expr) and isinstance(node.value, ast.Str) + if node.body: - if isinstance(node.body[0], ast.Expr) and \ - isinstance(node.body[0].value, ast.Str): - node.body.pop(0) + node.body = [child for child in node.body if not unused_string(child)] self.generic_visit(node) return node |