Skip to content
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

Add UML designs to developer documentation. #143

Merged
merged 4 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/sphinx.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ jobs:
${{ runner.os }}-pip-
${{ runner.os }}-

- name: Install dependencies
- name: Install system dependencies
run: sudo apt-get install plantuml

- name: Install Python dependencies
run: |
cd stylist
python -m pip install --upgrade pip
Expand Down
27 changes: 27 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,33 @@ As always it is also possible to install from the project source using
``pip install --editable .``. The source may be obtained by downloading a
tarball or by cloning the repository.

Development
-----------

If you are interested in developing this tool then a few useful prerequisite
lists are provided.

+--------------------------------+-------------------------------+
| Command | Tool Set |
+================================+===============================+
| ``pip install .[dev]`` | For development. |
+--------------------------------+-------------------------------+
| ``pip install .[test]`` | For testing. |
+--------------------------------+-------------------------------+
| ``pip install .[performance]`` | For monitoring performance. |
+--------------------------------+-------------------------------+
| ``pip install .[docs]`` | For generating documentation. |
+--------------------------------+-------------------------------+
| ``pip install .[release]`` | For creating releases. |
+--------------------------------+-------------------------------+

Note that the documenatation requires the `PlantUML`_ tool to be installed.

Until `PEP725`_ or similar is adopted this cannot be expressed in the
``pyproject.toml`` file so it is noted here.

.. _PlantUML: https://plantuml.com/
.. _PEP725: https://peps.python.org/pep-0725/

Usage
~~~~~
Expand Down
7 changes: 6 additions & 1 deletion documentation/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
'sphinx.ext.autosummary',
'sphinx_autodoc_typehints',
'sphinx.ext.todo',
'sphinx.ext.coverage'
'sphinx.ext.coverage',
'sphinxcontrib.plantuml'
]

# Paths to directories containing templates. Relative to this directory.
Expand All @@ -72,6 +73,10 @@
autoclass_content = 'both'


# -- Integrated PlantUML configuration ---------------------------------------

plantuml_output_format = 'svg'

# -- Todo configuration ------------------------------------------------------

todo_include_todos = True
Expand Down
46 changes: 46 additions & 0 deletions documentation/source/developer-information/design.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
.. _design-page:

Design
======

The fundamental building block of Stylist is the "Rule." Each rule implements
a single check which the source code must pass.

.. uml:: uml/rule_class.puml
:caption: UML class diagram showing "rule" module classes.
:align: center
:width: 100%

Rules are collected into "Styles" allowing different sets of rules to be used
for different purposes.

.. uml:: uml/style_class.puml
:caption: UML class diagram showing "style" module classes.
:align: center
:width: 100%

Source is presented either line-by-line as strings from the text file or as an
abstract syntax tree, gained by parsing the source.

.. uml:: uml/source_class.puml
:caption: UML class diagram showing "source" module classes.
:align: center
:width: 100%

The line based ``SourceText`` can come from either a file or a string buffer.
This text can have preprocessors applied to it using the decorator pattern.

Operation is orchestrated through the ``Engine`` class. It makes use of a
factory class to create the correct source object from a file.

.. uml:: uml/engine_class.puml
:caption: UML class diagram showing various orchestration classes.
:align: center
:width: 100%

Sample operation is shown in the following sequence diagram:

.. uml:: uml/sequence_diagram.puml
:caption: UML sequence diagram showing example operation.
:align: center
:width: 100%
50 changes: 50 additions & 0 deletions documentation/source/developer-information/future.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
Ideas For Future Work
=====================

Any ideas for substantial future work should be documented on this page.

Parse Tree Traversal Efficiency
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Every rule is individually responsible for traversing its search space, be
that line-by-line for textual rules or node-by-node for parse tree rules.

This means the lines of the source file or nodes of the parse tree are
visited by each rule in turn. This could be inefficient. There may be a
means of turning this inside out and have each line or node visited only
once and each rule see the entity in turn.

See issue `#46`_ for me details.

.. _#46: https://github.com/MetOffice/stylist/issues/46

Coerce Code
~~~~~~~~~~~

At the moment the tool is able to highlight problems with the source code.
This is useful but for a sub-set of problems there is an obvious remedy
which could be enacted. For instance, a missing ``intent`` specification could
be fixed by imposing a default, e.g. ``intent none``.

Issue `#57`_ has more details.

.. _#57: https://github.com/MetOffice/stylist/issues/57

The general design is very similar to the existing one, outlined in the
:ref:`design-page`. The difference being that ``Rule`` objects can mutate the
parse tree as they go.

It may be necessary to have failing and non-failing issues. In this case a code
coercion would be reported as a non-failing issue which may be viewed or
suppressed. Faults which cannot be coerced would still raise failing issues which
cannot be ignored.

It's not clear how line-by-line text rules would mutate the text form and how
the resulting parse tree would be regenerated after they had.

And an example of operation:

.. uml:: uml/future_sequence_diagram.puml
:caption: UML sequence diagram of example operation.
:align: center
:width: 100%
4 changes: 3 additions & 1 deletion documentation/source/developer-information/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ your taste.
:maxdepth: 2

overview
api
design
environment
rules
todo
future
api

A lot of our process is dictated by the GitHub platform used to host the
project. For details about that, such as how change requests are handled,
Expand Down
35 changes: 35 additions & 0 deletions documentation/source/developer-information/uml/engine_class.puml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
' stylist.source module

class source.FilePipe {
+<<create>>__init__(parser: SourceTree<<type>>, preprocessors: TextProcessor<<type>><<vararg>>)
}


source.FilePipe "*" --o source.SourceFactory
class source.SourceFactory {
+{class}add_extension_mapping(extension: String, pipe: source.FilePipe)
+{class}get_extensions(): String<<iterable>>
+{class}read_file(source_file: Path|TextIO): source.SourceTree
}

''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
' stylist.engine module

class engine.CheckEngine {
-styles: Style<<collection>>
+__init__( styles: Style<<collection>>)
+check( filename: String <<collection>> ): Issue<<collection>>
}
style.Style "+" -o engine.CheckEngine

''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
' stylist.issue module

class issue.Issue {
-description: String
-filename: Path <<optional>>
-line: Integer <<optional>>
+__init__( description: String, line: Integer <<optional>>, filename: Path <<optional>> )
+__str__(): String
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
@startuml Styler Sequence Diagram

participant UserInterface
participant engine.CheckEngine as CheckEngine
participant "source.SourceFactory" as SourceFactory
participant "fortran_source:source.FortranSource" as FortranSource
participant "my_style:style.LFRicstyle" as LFRicStyle
participant "missing_implict:rule.MissingImplicit" as MissingImplicit

activate SourceFactory

UserInterface -\ LFRicStyle : <<create>>
activate LFRicStyle

LFRicStyle -\ MissingImplicit : MissingImplicit(None) <<create>>
activate MissingImplicit

UserInterface -\ CheckEngine : CheckEngine(my_style)
activate CheckEngine

UserInterface -> SourceFactory : SourceFactory(filename)
SourceFactory -\ FortranSource : <<create>>
activate FortranSource
SourceFactory --> UserInterface : fortran_source

UserInterface -> CheckEngine : check(fortran_source)

CheckEngine -> LFRicStyle : check(fortran_source)

LFRicStyle -> MissingImplicit : examine(fortran_source)

alt "Implicit statement is missing"
MissingImplicit -\ FortranSource : insert_statement(Implicit(None))
end

MissingImplicit --> LFRicStyle : Issue <<sequence>>

LFRicStyle --> CheckEngine : Issue <<sequence>>

CheckEngine --> UserInterface : Issue <<sequence>>

@enduml
47 changes: 47 additions & 0 deletions documentation/source/developer-information/uml/rule_class.puml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
skinparam class {
BackgroundColor<<unwritten>> AlliceBlue
BorderColor<<unwritten>> LightSkyBlue
}
skinparam groupInheritance 2

abstract class rule.Rule {
+{abstract}examine(subject: source.SourceTree): issue.Issue<<collection>>
}

class rule.FortranCharacterset {
+examine( subject: source.SourceTree ): issue.Issue<<collection>>
}
rule.Rule <|-- rule.FortranCharacterset

class rule.TrailingWhitespace {
+examine( subject: source.SourceTree ): issue.Issue<<collection>>
}
rule.Rule ^-- rule.TrailingWhitespace

abstract class rule.FortranRule {
+examine(subject: source.SourceTree): issue.Issue<<collection>>
+{abstract}examine_fortran(subject: source.FortranSource): issue.Issue<<collection>>
}
rule.Rule <|--- rule.FortranRule

class rule.MissingVisibility <<unwritten>> {
+examine_fortran(subject: source.FortranSource): issue.Issue<<collection>>
}
rule.FortranRule <|-- rule.MissingVisibility

class rule.MissingImplicit {
+<<create>>__init__(require_everywhere: Bool = False)
+examine_fortran(subject: source.FortranSource): issue.Issue<<collection>>
}
rule.FortranRule <|-- rule.MissingImplicit

class rule.MissingLegalNotice <<unwritten>> {
+examine_fortran(subject: source.FortranSource): issue.Issue<<collection>>
}
rule.FortranRule <|-- rule.MissingLegalNotice

abstract class rule.CRule <<unwritten>> {
+examine( subject: SourceTree ): Issue<<collection>>
+{abstract}examine_c(subject: source.CSource): issue.Issue<collection>
}
rule.Rule <|-- rule.CRule
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
@startuml Checker Sequence Diagram
participant UserInterface
participant engine.CheckEngine as CheckEngine
participant "my_style:rule.LFRicStyle" as LFRicStyle
participant "rule.MissingImplicit" as MissingImplicit
participant source.SourceFactory as SourceFactory
participant "some_text:source.SourceFileReader" as SourceFileReader
participant "source_tree:source.FortranSource" as FortranSource

activate SourceFactory

UserInterface -\ LFRicStyle : LFRicStyle() <<create>>
activate LFRicStyle

LFRicStyle -\ MissingImplicit : <<create>>
activate MissingImplicit

UserInterface -\ CheckEngine : CheckEngine(my_style) <<create>>
activate CheckEngine

UserInterface -> SourceFactory: read_file(filename)

SourceFactory -\ SourceFileReader: SourceFileReader(filename) <<create>>
activate SourceFileReader

SourceFactory -\ FortranSource: FortranSource(some_text) <<create>>
activate FortranSource

SourceFactory --> UserInterface: source_tree

UserInterface -> CheckEngine : check(source_tree)

CheckEngine -> FortranSource : get_tree()
FortranSource --> CheckEngine : root:Program

CheckEngine -> LFRicStyle : check(root)
LFRicStyle -> MissingImplicit : examine(root)
MissingImplicit --> LFRicStyle : Issues[]
LFRicStyle --> CheckEngine : Issues[]

CheckEngine --> UserInterface : Issues[]

@enduml
Loading
Loading