Skip to content

Refactor pyerverse Association Logic #10397

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Julfried
Copy link
Contributor

@Julfried Julfried commented May 18, 2025

Type of Changes

Type
βœ“ πŸ› Bug fix
βœ“ πŸ”¨ Refactoring

Description

Closes #9045

Summary

Fixed incorrect UML semantics in pyreverse for aggregation vs composition relationships. The previous implementation had no explicit CompositionsHandler class and did not differentiate between Aggregation and Composition properly, treating most relationships as aggregation by default.

Problem

Pyreverse previously lacked proper UML relationship semantics. So I would proposes the following distinction for Python class relationships:

fields.py

# Test for https://github.com/pylint-dev/pylint/issues/9045

class P:
    pass

class A:
    x: P  # just type hint, no ownership β†’ Association

class B:
    def __init__(self, x: P):
        self.x = x  # receives object, not created β†’ Aggregation

class C:
    x: P
    def __init__(self, x: P):
        self.x = x  # receives object, not created β†’ Aggregation

class D:
    x: P
    def __init__(self):
        self.x = P()  # creates object β†’ Composition

class E:
    def __init__(self):
        self.x = P()  # creates object β†’ Composition

fields.mmd

classDiagram
  class A {
    x
  }
  class B {
    x
  }
  class C {
    x
  }
  class D {
    x
  }
  class E {
    x
  }
  class P {
  }
  P --> A : x
  P --* D : x
  P --* E : x
  P --o B : x
  P --o C : x

Changes Made

1. Added CompositionsHandler

Created dedicated handler following chain of responsibility pattern:
CompositionsHandler β†’ AggregationsHandler β†’ AssociationsHandler

2. Updated Printers

  • Added EdgeType.COMPOSITION enum value
  • Updated MermaidJS and PlantUML printers with correct arrows per Mermaid docs
  • For the Dot printer I used plain arrows, but I did not manage to find something in the dot language documentation

3. Prevented Duplicate Relationships

Modified extract_relationships() to process relationships by priority and avoid duplicates.

Currently I only made the modifications needed to implement the new Associations Logic. If you like the proposed distinction I would work on making the tests pass :)

@Julfried Julfried requested a review from DudeNr33 as a code owner May 18, 2025 23:51
Copy link
Contributor

πŸ€– According to the primer, this change has no effect on the checked open source code. πŸ€–πŸŽ‰

This comment was generated for commit d0ea86e

@Pierre-Sassoulas Pierre-Sassoulas added pyreverse Related to pyreverse component Enhancement ✨ Improvement to a component Work in progress labels May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component pyreverse Related to pyreverse component Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pyreverse: composition / aggregation arrow strange behavior (and field annotation bug)
2 participants