Skip to content

Too easy to trigger duplicate-code with multiline destructuring assignments #10319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ruro opened this issue Mar 29, 2025 · 1 comment
Closed
Labels
Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling

Comments

@ruro
Copy link
Contributor

ruro commented Mar 29, 2025

Bug description

Consider the following:

mkdir -p example
touch example/__init__.py

cat >example/common.py <<'EOF'
def this_code_is_already_factored_out(a, b):
    del a, b
    return "a", "b", "c", "d"
EOF

cat >example/bar1.py <<'EOF'
from .common import this_code_is_already_factored_out


class Bar1:
    a_lot_of_attributes = 1
    with_long_names = 2
    that_dont_fit = 3
    on_one_line = 4

    def different(self):
        return 0

    def bee(self):
        (
            self.a_lot_of_attributes,
            self.with_long_names,
            self.that_dont_fit,
            self.on_one_line,
        ) = this_code_is_already_factored_out(
            a=1_000_000_000 + 0.000_000_000_1,
            b={2_000_000_000, 3_000_000_000},
        )
EOF

cat >example/bar2.py <<'EOF'
from .common import this_code_is_already_factored_out


class Bar2:
    a_lot_of_attributes = "this"
    with_long_names = "code"
    that_dont_fit = "is"
    on_one_line = "different"

    def woop(self):
        (
            self.a_lot_of_attributes,
            self.with_long_names,
            self.that_dont_fit,
            self.on_one_line,
        ) = this_code_is_already_factored_out(
            a=["the arguments are also long enough"],
            b="so" + "they" + "dont" + "fit" + "on" + "one" + "line",
        )

    def unrelated(self):
        return "clearly different"
EOF

It's a bit silly to treat such multiline destructuring assignments as "lines" for the purposes of duplicate-code / min-similarity-lines. In particular, I am struggling to come up with a way to refactor this kind of code to "satisfy" pylint.

In this case, all the common logic has already been refactored into a separate function and the only thing left is the assignment of the attributes. This can't be fixed by using inheritance, because the woop and bee methods themselves are different (and the Baz1/Baz2 classes are barely related).

I suppose, I could create a separate function

def run_common_code_and_assign(target, a, b):
    (
        target.a_lot_of_attributes,
        target.with_long_names,
        target.that_dont_fit,
        target.on_one_line,
    ) = this_code_is_already_factored_out(
        a=a,
        b=b,
    )

and then use it like

class Bar1:
    def bee(self):
        run_common_code_and_assign(
            target=self,
            a=1_000_000_000 + 0.000_000_000_1,
            b={2_000_000_000, 3_000_000_000},
        )

class Bar2:
    def woop(self):
        run_common_code_and_assign(
            target=self,
            a=["the arguments are also long enough"],
            b="so" + "they" + "dont" + "fit" + "on" + "one" + "line",
        )

However, this essentially makes target an "out parameter" for run_common_code_and_assign, which is an antipattern in Python (and setting class members outside the class/module is also kind of yucky, especially if a_lot_of_attributes are _protected or __private).

Command used

pylint --rcfile= --disable=missing-docstring example

Pylint output

************* Module example.common
example/common.py:1:0: R0801: Similar lines in 2 files
==example.bar1:[13:19]
==example.bar2:[10:16]
        (
            self.a_lot_of_attributes,
            self.with_long_names,
            self.that_dont_fit,
            self.on_one_line,
        ) = this_code_is_already_factored_out( (duplicate-code)

------------------------------------------------------------------
Your code has been rated at 9.57/10 (previous run: 9.57/10, +0.00)

Expected behavior

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Pylint version

pylint 3.3.6
astroid 3.3.9
Python 3.12.8 (main, Dec  3 2024, 18:42:41) [GCC 14.2.1 20241116]
@ruro ruro added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Mar 29, 2025
@ruro
Copy link
Contributor Author

ruro commented Mar 29, 2025

Sorry. This seems to be a duplicate of #7920.

I originally missed that issue, because I thought that this issue was triggered because of the destructuring assignment specifically instead of any expression/statement/function call.

@ruro ruro closed this as completed Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling
Projects
None yet
Development

No branches or pull requests

1 participant