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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
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
37 changes: 37 additions & 0 deletions documentation/source/developer-information/design.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
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

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%

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

Sample operation is shown in the following sequence diagram:

.. uml:: uml/sequence_diagram.puml
:caption: UML sequence diagram showing example operation.
41 changes: 41 additions & 0 deletions documentation/source/developer-information/future.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
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 `intent none`.

Issue `#57`_ has more details.

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

A proposed design is given here:

.. uml:: uml/future_class_diagram.puml
:caption: UML class diagram of potential coercive implementation.

And an example of operation:

.. uml:: uml/future_sequence_diagram.puml
:caption: UML sequence diagram of example operation.
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
33 changes: 33 additions & 0 deletions documentation/source/developer-information/uml/engine_class.puml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
' 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
+__init__( description: String )
+__str__(): String
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,31 +89,3 @@ class issue.Issue {
}

@enduml


@startuml Styler Sequence Diagram

participant UserInterface

UserInterface -> style.LFRicStyle : <<instantiate>>
activate style.LFRicStyle

style.LFRicStyle -> rule.MissingImplicit : MissingImplicit(None)
activate rule.MissingImplicit

UserInterface -> engine.CheckEngine : CheckEngine(LFRicStyle)
activate engine.CheckEngine

UserInterface -> engine.CheckEngine : check(SourceFile)

engine.CheckEngine -> style.LFRicStyle : check(Program)

style.LFRicStyle -> style.MissingImplicit : examine(Program)

style.MissingImplicit --> style.Style : Issues[]

style.Style --> engine.CheckEngine : Issues[]

engine.CheckEngine --> UserInterface : Issues[]

@enduml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
@startuml Styler Sequence Diagram

participant UserInterface

UserInterface -> style.LFRicStyle : <<instantiate>>
activate style.LFRicStyle

style.LFRicStyle -> rule.MissingImplicit : MissingImplicit(None)
activate rule.MissingImplicit

UserInterface -> engine.CheckEngine : CheckEngine(LFRicStyle)
activate engine.CheckEngine

UserInterface -> engine.CheckEngine : check(SourceFile)

engine.CheckEngine -> style.LFRicStyle : check(Program)

style.LFRicStyle -> style.MissingImplicit : examine(Program)

style.MissingImplicit --> style.Style : Issues[]

style.Style --> engine.CheckEngine : Issues[]

engine.CheckEngine --> UserInterface : Issues[]

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

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

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

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,25 @@
@startuml Checker Sequence Diagram
participant UserInterface

UserInterface -> style.LFRicStyle : <<instantiate>>
activate style.LFRicStyle

style.LFRicStyle -> rule.MissingImplicit : MissingImplicit(None)
activate rule.MissingImplicit

UserInterface -> engine.CheckEngine : CheckEngine(LFRicStyle)
activate engine.CheckEngine

UserInterface -> engine.CheckEngine : check(SourceFile)

engine.CheckEngine -> style.LFRicStyle : check(Program)

style.LFRicStyle -> rule.MissingImplicit : examine(Program)

rule.MissingImplicit --> style.Style : Issues[]

style.Style --> engine.CheckEngine : Issues[]

engine.CheckEngine --> UserInterface : Issues[]

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


abstract class source.SourceText {
+{abstract}get_text(): String
}


source.TextProcessor *-- source.SourceText
abstract class source.TextProcessor {
+<<create>>__init__(source: SourceText)
+{abstract}{static}get_name(): String
+{abstract}get_text(): String
}
source.SourceText <|-- source.TextProcessor


class source.CPreProcessor {
+{static}get_name(): String
+get_text(): String
}
source.TextProcessor <|-- source.CPreProcessor


class source.FortranPreProcessor {
+{static}get_name(): String
+get_text(): String
}
source.TextProcessor <|-- source.FortranPreProcessor


class source.pFUnitProcessor {
+{static}get_name(): String
+get_text(): String
}
source.TextProcessor <|-- source.pFUnitProcessor


class source.FileReader {
+<<create>>__init__(source_file: Path|TextIO)
+get_text(): String
}
source.SourceText <|-- source.FileReader


class source.StringReader {
+<<create>>__init__(source_string: String)
+get_text(): String
}
source.SourceText <|-- source.StringReader


source.SourceTree *-- source.SourceText
abstract class source.SourceTree {
+<<create>>__init__( text: SourceText )
+{abstract}get_tree()
+{abstract}get_tree_error(): String
+get_text(): String
}


class source.FortranSource {
+get_tree(): fparser.Fortran2003.Program
+get_tree_error(): String
+get_first_statement(root: fparser.Fortran2003.Block): fparser.Fortran2003.StmtBlock
+path( path: String, root: root: fparser.Fortran2003.Block ): fparser.Fortran2003.Base<<list>>
+find_all(find_node: fparser.Fortran2003.Base<<type>>, root: fparser.Fortran2003.Base): fparser.Fortran2003.Base<<list>>
+{static}print_tree(root: fparser.Fortran2003.Base, indent: Integer = 0)
}
source.SourceTree <|-- source.FortranSource


class source.CSource <<unwritten>> {
+get_tree()
+get_tree_error()
}
source.SourceTree <|-- source.CSource


class source.PlainText {
+{static}get_name(): String
+get_tree(): String<<generator>>
+get_tree_error(): String<<optional>>
}
source.SourceTree <|-- source.PlainText
Loading
Loading