From ad8eb4d3bff6adea995b03eb366b2ed164f67066 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Thu, 21 Dec 2023 10:50:22 -0300 Subject: [PATCH 1/4] Added comma inference to setup py writer --- .../dependency_management/setup_py_writer.py | 21 +++++-- .../test_setup_py_writer.py | 61 ++++++++++++++++++- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/codemodder/dependency_management/setup_py_writer.py b/src/codemodder/dependency_management/setup_py_writer.py index cbc7956a..1b192be7 100644 --- a/src/codemodder/dependency_management/setup_py_writer.py +++ b/src/codemodder/dependency_management/setup_py_writer.py @@ -110,15 +110,28 @@ def add_dependencies_to_arg(self, arg: cst.Arg) -> cst.Arg: # dependency listed in install_requires self.line_num_changed = self.lineno_for_node(arg.value.elements[-1]) - 1 + # grab the penultimate comma value, or the last one if it has a single element + last_element = arg.value.elements[-1] + new_comma = cst.Comma(whitespace_after=cst.SimpleWhitespace(" ")) + if len(arg.value.elements) > 1: + new_comma = arg.value.elements[-2].comma + + # last new dependency will not have the new comma value new_dependencies = [ - cst.Element(value=cst.SimpleString(value=f'"{str(dep)}"')) - for dep in self.dependencies + cst.Element(value=cst.SimpleString(value=f'"{str(dep)}"'), comma=new_comma) + for dep in self.dependencies[:-1] + ] + [ + cst.Element(value=cst.SimpleString(value=f'"{str(self.dependencies[-1])}"')) ] - # TODO: detect if elements are separated by newline in source code. + return cst.Arg( keyword=arg.keyword, value=arg.value.with_changes( - elements=arg.value.elements + tuple(new_dependencies) + elements=[ + *arg.value.elements[:-1], + last_element.with_changes(comma=new_comma), + *new_dependencies, + ] ), equal=arg.equal, comma=arg.comma, diff --git a/tests/dependency_management/test_setup_py_writer.py b/tests/dependency_management/test_setup_py_writer.py index bda76d93..44233df7 100644 --- a/tests/dependency_management/test_setup_py_writer.py +++ b/tests/dependency_management/test_setup_py_writer.py @@ -11,6 +11,57 @@ TEST_DEPENDENCIES = [Requirement("defusedxml==0.7.1"), Requirement("security~=1.2.0")] +def test_update_setuppy_comma_default(tmpdir): + original = """ + from setuptools import setup + setup( + name="test pkg", + description="testing", + long_description="...", + author="Pixee", + packages=find_packages("src"), + package_dir={"": "src"}, + python_requires=">3.6", + install_requires=[ + "protobuf>=3.12,<3.18; python_version < '3'", + ], + entry_points={}, + ) + """ + + dependency_file = tmpdir.join("setup.py") + dependency_file.write(dedent(original)) + + store = PackageStore( + type=FileType.SETUP_PY, + file=str(dependency_file), + dependencies=set(), + py_versions=[">=3.6"], + ) + + writer = SetupPyWriter(store, tmpdir) + dependencies = [DefusedXML, Security] + writer.write(dependencies, dry_run=False) + + after = """ + from setuptools import setup + setup( + name="test pkg", + description="testing", + long_description="...", + author="Pixee", + packages=find_packages("src"), + package_dir={"": "src"}, + python_requires=">3.6", + install_requires=[ + "protobuf>=3.12,<3.18; python_version < '3'", "defusedxml~=0.7.1", "security~=1.2.0" + ], + entry_points={}, + ) + """ + assert dependency_file.read() == dedent(after) + + @pytest.mark.parametrize("dry_run", [True, False]) def test_update_setuppy_dependencies(tmpdir, dry_run): original = """ @@ -61,7 +112,9 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): "protobuf>=3.12,<3.18; python_version < '3'", "protobuf>=3.12,<4; python_version >= '3'", "psutil>=5.7,<6", - "requests>=2.4.2,<3", "defusedxml~=0.7.1", "security~=1.2.0" + "requests>=2.4.2,<3", + "defusedxml~=0.7.1", + "security~=1.2.0" ], entry_points={}, ) @@ -73,12 +126,14 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): res = ( "--- \n" "+++ \n" - "@@ -12,7 +12,7 @@\n" + "@@ -12,7 +12,9 @@\n" """ "protobuf>=3.12,<3.18; python_version < '3'",\n""" """ "protobuf>=3.12,<4; python_version >= '3'",\n""" """ "psutil>=5.7,<6",\n""" """- "requests>=2.4.2,<3"\n""" - """+ "requests>=2.4.2,<3", "defusedxml~=0.7.1", "security~=1.2.0"\n""" + """+ "requests>=2.4.2,<3",\n""" + """+ "defusedxml~=0.7.1",\n""" + """+ "security~=1.2.0"\n""" " ],\n " " entry_points={},\n" " )\n" From 45ddc5eeb74d81d3292a47eda35c2fbe3f25d7e4 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Fri, 22 Dec 2023 07:09:38 -0300 Subject: [PATCH 2/4] Added newline inference when adding dependencies in setup.py --- .../dependency_management/setup_py_writer.py | 12 ++++- .../test_setup_py_writer.py | 53 ++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/codemodder/dependency_management/setup_py_writer.py b/src/codemodder/dependency_management/setup_py_writer.py index 1b192be7..6f3af56a 100644 --- a/src/codemodder/dependency_management/setup_py_writer.py +++ b/src/codemodder/dependency_management/setup_py_writer.py @@ -110,11 +110,21 @@ def add_dependencies_to_arg(self, arg: cst.Arg) -> cst.Arg: # dependency listed in install_requires self.line_num_changed = self.lineno_for_node(arg.value.elements[-1]) - 1 - # grab the penultimate comma value, or the last one if it has a single element last_element = arg.value.elements[-1] new_comma = cst.Comma(whitespace_after=cst.SimpleWhitespace(" ")) + # grab the penultimate comma value if it has more than one element if len(arg.value.elements) > 1: new_comma = arg.value.elements[-2].comma + else: + # infer the indentation from lbracket, if any + match arg.value.lbracket.whitespace_after: + case cst.ParenthesizedWhitespace(): + new_comma = cst.Comma( + whitespace_after=cst.ParenthesizedWhitespace( + indent=True, + last_line=arg.value.lbracket.whitespace_after.last_line, + ) + ) # last new dependency will not have the new comma value new_dependencies = [ diff --git a/tests/dependency_management/test_setup_py_writer.py b/tests/dependency_management/test_setup_py_writer.py index 44233df7..9bc85539 100644 --- a/tests/dependency_management/test_setup_py_writer.py +++ b/tests/dependency_management/test_setup_py_writer.py @@ -11,7 +11,7 @@ TEST_DEPENDENCIES = [Requirement("defusedxml==0.7.1"), Requirement("security~=1.2.0")] -def test_update_setuppy_comma_default(tmpdir): +def test_update_setuppy_comma_single_element_newline(tmpdir): original = """ from setuptools import setup setup( @@ -54,7 +54,9 @@ def test_update_setuppy_comma_default(tmpdir): package_dir={"": "src"}, python_requires=">3.6", install_requires=[ - "protobuf>=3.12,<3.18; python_version < '3'", "defusedxml~=0.7.1", "security~=1.2.0" + "protobuf>=3.12,<3.18; python_version < '3'", + "defusedxml~=0.7.1", + "security~=1.2.0" ], entry_points={}, ) @@ -62,6 +64,53 @@ def test_update_setuppy_comma_default(tmpdir): assert dependency_file.read() == dedent(after) +def test_update_setuppy_comma_single_element_inline(tmpdir): + original = """ + from setuptools import setup + setup( + name="test pkg", + description="testing", + long_description="...", + author="Pixee", + packages=find_packages("src"), + package_dir={"": "src"}, + python_requires=">3.6", + install_requires=["protobuf>=3.12,<3.18; python_version < '3'",], + entry_points={}, + ) + """ + + dependency_file = tmpdir.join("setup.py") + dependency_file.write(dedent(original)) + + store = PackageStore( + type=FileType.SETUP_PY, + file=str(dependency_file), + dependencies=set(), + py_versions=[">=3.6"], + ) + + writer = SetupPyWriter(store, tmpdir) + dependencies = [DefusedXML, Security] + writer.write(dependencies, dry_run=False) + + after = """ + from setuptools import setup + setup( + name="test pkg", + description="testing", + long_description="...", + author="Pixee", + packages=find_packages("src"), + package_dir={"": "src"}, + python_requires=">3.6", + install_requires=["protobuf>=3.12,<3.18; python_version < '3'", "defusedxml~=0.7.1", "security~=1.2.0"], + entry_points={}, + ) + """ + assert dependency_file.read() == dedent(after) + + @pytest.mark.parametrize("dry_run", [True, False]) def test_update_setuppy_dependencies(tmpdir, dry_run): original = """ From 2b264547cea0f28223edf0a482e02ec40a96da06 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 2 Jan 2024 09:20:02 -0300 Subject: [PATCH 3/4] Handled cases for commas in the last elements of dependencies --- .../dependency_management/setup_py_writer.py | 38 +++++++++++++++---- .../test_setup_py_writer.py | 4 +- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/codemodder/dependency_management/setup_py_writer.py b/src/codemodder/dependency_management/setup_py_writer.py index 6f3af56a..f568b21f 100644 --- a/src/codemodder/dependency_management/setup_py_writer.py +++ b/src/codemodder/dependency_management/setup_py_writer.py @@ -110,29 +110,51 @@ def add_dependencies_to_arg(self, arg: cst.Arg) -> cst.Arg: # dependency listed in install_requires self.line_num_changed = self.lineno_for_node(arg.value.elements[-1]) - 1 - last_element = arg.value.elements[-1] - new_comma = cst.Comma(whitespace_after=cst.SimpleWhitespace(" ")) # grab the penultimate comma value if it has more than one element + new_comma = cst.Comma(whitespace_after=cst.SimpleWhitespace(" ")) + last_element = arg.value.elements[-1] + new_last_element = cst.Element( + value=cst.SimpleString(value=f'"{str(self.dependencies[-1])}"') + ) if len(arg.value.elements) > 1: new_comma = arg.value.elements[-2].comma + # if it has a newline, add a comma to the last element + # this follows black's standard + match new_comma: + case cst.ParenthesizedWhitespace(newline=cst.Newline()): + new_last_element = new_last_element.with_changes(comma=cst.Comma()) else: # infer the indentation from lbracket, if any - match arg.value.lbracket.whitespace_after: + match lbracket_whitespace := arg.value.lbracket.whitespace_after: case cst.ParenthesizedWhitespace(): new_comma = cst.Comma( whitespace_after=cst.ParenthesizedWhitespace( indent=True, - last_line=arg.value.lbracket.whitespace_after.last_line, + last_line=lbracket_whitespace.last_line, ) ) + # In a list with a single element, libcst will attribute the newline to the rbracket + match arg.value.rbracket.whitespace_before.first_line: + case cst.ParenthesizedWhitespace( + newline=cst.Newline() + ) | cst.TrailingWhitespace(newline=cst.Newline()): + new_last_element = new_last_element.with_changes( + comma=cst.Comma() + ) # last new dependency will not have the new comma value new_dependencies = [ cst.Element(value=cst.SimpleString(value=f'"{str(dep)}"'), comma=new_comma) for dep in self.dependencies[:-1] - ] + [ - cst.Element(value=cst.SimpleString(value=f'"{str(self.dependencies[-1])}"')) - ] + ] + [new_last_element] + + # enforce newline at the rbracket if multiline + # this is the way cst naturally parses, instead of attributing it to the Comma + # new_rbracket = arg.value.rbracket + # if len(arg.value.elements) > 1: + # match new_comma.whitespace_after: + # case cst.ParenthesizedWhitespace(): + # new_rbracket = new_rbracket.with_changes(newline = new_comma.whitespace_after.first_line.newline) return cst.Arg( keyword=arg.keyword, @@ -141,7 +163,7 @@ def add_dependencies_to_arg(self, arg: cst.Arg) -> cst.Arg: *arg.value.elements[:-1], last_element.with_changes(comma=new_comma), *new_dependencies, - ] + ], ), equal=arg.equal, comma=arg.comma, diff --git a/tests/dependency_management/test_setup_py_writer.py b/tests/dependency_management/test_setup_py_writer.py index 9bc85539..e6bf3d6e 100644 --- a/tests/dependency_management/test_setup_py_writer.py +++ b/tests/dependency_management/test_setup_py_writer.py @@ -56,7 +56,7 @@ def test_update_setuppy_comma_single_element_newline(tmpdir): install_requires=[ "protobuf>=3.12,<3.18; python_version < '3'", "defusedxml~=0.7.1", - "security~=1.2.0" + "security~=1.2.0", ], entry_points={}, ) @@ -75,7 +75,7 @@ def test_update_setuppy_comma_single_element_inline(tmpdir): packages=find_packages("src"), package_dir={"": "src"}, python_requires=">3.6", - install_requires=["protobuf>=3.12,<3.18; python_version < '3'",], + install_requires=["protobuf>=3.12,<3.18; python_version < '3'"], entry_points={}, ) """ From f3e42b3109126ff469ead08511c8e3f996b4ca8d Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 2 Jan 2024 13:23:55 -0300 Subject: [PATCH 4/4] Bugfix for comma in multi-line dependencies list --- .../dependency_management/setup_py_writer.py | 19 ++++----------- .../test_setup_py_writer.py | 24 ++++++++++--------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/codemodder/dependency_management/setup_py_writer.py b/src/codemodder/dependency_management/setup_py_writer.py index f568b21f..50eab85a 100644 --- a/src/codemodder/dependency_management/setup_py_writer.py +++ b/src/codemodder/dependency_management/setup_py_writer.py @@ -120,8 +120,10 @@ def add_dependencies_to_arg(self, arg: cst.Arg) -> cst.Arg: new_comma = arg.value.elements[-2].comma # if it has a newline, add a comma to the last element # this follows black's standard - match new_comma: - case cst.ParenthesizedWhitespace(newline=cst.Newline()): + match new_comma.whitespace_after: + case cst.ParenthesizedWhitespace( + first_line=cst.TrailingWhitespace(newline=cst.Newline()) + ): new_last_element = new_last_element.with_changes(comma=cst.Comma()) else: # infer the indentation from lbracket, if any @@ -135,27 +137,16 @@ def add_dependencies_to_arg(self, arg: cst.Arg) -> cst.Arg: ) # In a list with a single element, libcst will attribute the newline to the rbracket match arg.value.rbracket.whitespace_before.first_line: - case cst.ParenthesizedWhitespace( - newline=cst.Newline() - ) | cst.TrailingWhitespace(newline=cst.Newline()): + case cst.TrailingWhitespace(newline=cst.Newline()): new_last_element = new_last_element.with_changes( comma=cst.Comma() ) - # last new dependency will not have the new comma value new_dependencies = [ cst.Element(value=cst.SimpleString(value=f'"{str(dep)}"'), comma=new_comma) for dep in self.dependencies[:-1] ] + [new_last_element] - # enforce newline at the rbracket if multiline - # this is the way cst naturally parses, instead of attributing it to the Comma - # new_rbracket = arg.value.rbracket - # if len(arg.value.elements) > 1: - # match new_comma.whitespace_after: - # case cst.ParenthesizedWhitespace(): - # new_rbracket = new_rbracket.with_changes(newline = new_comma.whitespace_after.first_line.newline) - return cst.Arg( keyword=arg.keyword, value=arg.value.with_changes( diff --git a/tests/dependency_management/test_setup_py_writer.py b/tests/dependency_management/test_setup_py_writer.py index e6bf3d6e..0fa919f8 100644 --- a/tests/dependency_management/test_setup_py_writer.py +++ b/tests/dependency_management/test_setup_py_writer.py @@ -127,7 +127,7 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): "protobuf>=3.12,<3.18; python_version < '3'", "protobuf>=3.12,<4; python_version >= '3'", "psutil>=5.7,<6", - "requests>=2.4.2,<3" + "requests>=2.4.2,<3", ], entry_points={}, ) @@ -163,7 +163,7 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): "psutil>=5.7,<6", "requests>=2.4.2,<3", "defusedxml~=0.7.1", - "security~=1.2.0" + "security~=1.2.0", ], entry_points={}, ) @@ -175,14 +175,12 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): res = ( "--- \n" "+++ \n" - "@@ -12,7 +12,9 @@\n" - """ "protobuf>=3.12,<3.18; python_version < '3'",\n""" + "@@ -13,6 +13,8 @@\n" """ "protobuf>=3.12,<4; python_version >= '3'",\n""" """ "psutil>=5.7,<6",\n""" - """- "requests>=2.4.2,<3"\n""" - """+ "requests>=2.4.2,<3",\n""" + """ "requests>=2.4.2,<3",\n""" """+ "defusedxml~=0.7.1",\n""" - """+ "security~=1.2.0"\n""" + """+ "security~=1.2.0",\n""" " ],\n " " entry_points={},\n" " )\n" @@ -215,7 +213,7 @@ def test_other_setup_func(tmpdir): "protobuf>=3.12,<3.18; python_version < '3'", "protobuf>=3.12,<4; python_version >= '3'", "psutil>=5.7,<6", - "requests>=2.4.2,<3" + "requests>=2.4.2,<3", ], entry_points={}, ) @@ -253,7 +251,7 @@ def test_not_setup_file(tmpdir): "protobuf>=3.12,<3.18; python_version < '3'", "protobuf>=3.12,<4; python_version >= '3'", "psutil>=5.7,<6", - "requests>=2.4.2,<3" + "requests>=2.4.2,<3", ], entry_points={}, ) @@ -414,7 +412,9 @@ def test_setup_call_requirements_separate(tmpdir): "protobuf>=3.12,<3.18; python_version < '3'", "protobuf>=3.12,<4; python_version >= '3'", "psutil>=5.7,<6", - "requests>=2.4.2,<3", "defusedxml==0.7.1", "security~=1.2.0" + "requests>=2.4.2,<3", + "defusedxml==0.7.1", + "security~=1.2.0", ] setup( name="test pkg", @@ -440,7 +440,9 @@ def test_setup_call_requirements_separate(tmpdir): """ "protobuf>=3.12,<4; python_version >= '3'",\n""" """ "psutil>=5.7,<6",\n""" """- "requests>=2.4.2,<3"\n""" - """+ "requests>=2.4.2,<3", "defusedxml~=0.7.1", "security~=1.2.0"\n""" + """+ "requests>=2.4.2,<3",\n""" + """+ "defusedxml~=0.7.1",\n""" + """+ "security~=1.2.0",\n""" " ],\n " " entry_points={},\n" " )\n"