From 6329d9a809123361733b5ffa669fff605052bed4 Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Thu, 17 Oct 2024 09:19:57 +0100 Subject: [PATCH] First draft. --- .github/workflows/sphinx.yml | 5 +- documentation/source/conf.py | 7 +- .../source/developer-information/design.rst | 37 ++++ .../source/developer-information/future.rst | 41 ++++ .../source/developer-information/index.rst | 4 +- .../uml/engine_class.puml | 33 +++ .../uml/future_class_diagram.puml} | 28 --- .../uml/future_sequence_diagram.puml | 26 +++ .../developer-information/uml/rule_class.puml | 51 +++++ .../uml/sequence_diagram.puml | 25 +++ .../uml/source_class.puml | 88 ++++++++ .../uml/style_class.puml | 30 +++ documentation/uml/CheckerDesign.puml | 197 ------------------ pyproject.toml | 2 + 14 files changed, 346 insertions(+), 228 deletions(-) create mode 100644 documentation/source/developer-information/design.rst create mode 100644 documentation/source/developer-information/future.rst create mode 100644 documentation/source/developer-information/uml/engine_class.puml rename documentation/{uml/StylerDesign.puml => source/developer-information/uml/future_class_diagram.puml} (75%) create mode 100644 documentation/source/developer-information/uml/future_sequence_diagram.puml create mode 100644 documentation/source/developer-information/uml/rule_class.puml create mode 100644 documentation/source/developer-information/uml/sequence_diagram.puml create mode 100644 documentation/source/developer-information/uml/source_class.puml create mode 100644 documentation/source/developer-information/uml/style_class.puml delete mode 100644 documentation/uml/CheckerDesign.puml diff --git a/.github/workflows/sphinx.yml b/.github/workflows/sphinx.yml index 9dafb21..50399d7 100644 --- a/.github/workflows/sphinx.yml +++ b/.github/workflows/sphinx.yml @@ -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 diff --git a/documentation/source/conf.py b/documentation/source/conf.py index 133722b..b228dc5 100644 --- a/documentation/source/conf.py +++ b/documentation/source/conf.py @@ -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. @@ -72,6 +73,10 @@ autoclass_content = 'both' +# -- Integrated PlantUML configuration --------------------------------------- + +plantuml_output_format = 'svg' + # -- Todo configuration ------------------------------------------------------ todo_include_todos = True diff --git a/documentation/source/developer-information/design.rst b/documentation/source/developer-information/design.rst new file mode 100644 index 0000000..906288b --- /dev/null +++ b/documentation/source/developer-information/design.rst @@ -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. diff --git a/documentation/source/developer-information/future.rst b/documentation/source/developer-information/future.rst new file mode 100644 index 0000000..3901fbc --- /dev/null +++ b/documentation/source/developer-information/future.rst @@ -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. diff --git a/documentation/source/developer-information/index.rst b/documentation/source/developer-information/index.rst index b372fae..26c0461 100644 --- a/documentation/source/developer-information/index.rst +++ b/documentation/source/developer-information/index.rst @@ -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, diff --git a/documentation/source/developer-information/uml/engine_class.puml b/documentation/source/developer-information/uml/engine_class.puml new file mode 100644 index 0000000..300b385 --- /dev/null +++ b/documentation/source/developer-information/uml/engine_class.puml @@ -0,0 +1,33 @@ +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' +' stylist.source module + +class source.FilePipe { + +<>__init__(parser: SourceTree<>, preprocessors: TextProcessor<><>) +} + + +source.FilePipe "*" --o source.SourceFactory +class source.SourceFactory { + +{class}add_extension_mapping(extension: String, pipe: source.FilePipe) + +{class}get_extensions(): String<> + +{class}read_file(source_file: Path|TextIO): source.SourceTree +} + +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' +' stylist.engine module + +class engine.CheckEngine { + -styles: Style<> + +__init__( styles: Style<>) + +check( filename: String <> ): Issue<> + } +style.Style "+" -o engine.CheckEngine + +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' +' stylist.issue module + +class issue.Issue { + -description: String + +__init__( description: String ) + +__str__(): String + } diff --git a/documentation/uml/StylerDesign.puml b/documentation/source/developer-information/uml/future_class_diagram.puml similarity index 75% rename from documentation/uml/StylerDesign.puml rename to documentation/source/developer-information/uml/future_class_diagram.puml index c066542..a0d15bb 100644 --- a/documentation/uml/StylerDesign.puml +++ b/documentation/source/developer-information/uml/future_class_diagram.puml @@ -89,31 +89,3 @@ class issue.Issue { } @enduml - - -@startuml Styler Sequence Diagram - -participant UserInterface - -UserInterface -> style.LFRicStyle : <> -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 diff --git a/documentation/source/developer-information/uml/future_sequence_diagram.puml b/documentation/source/developer-information/uml/future_sequence_diagram.puml new file mode 100644 index 0000000..a8404a9 --- /dev/null +++ b/documentation/source/developer-information/uml/future_sequence_diagram.puml @@ -0,0 +1,26 @@ +@startuml Styler Sequence Diagram + +participant UserInterface + +UserInterface -> style.LFRicStyle : <> +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 diff --git a/documentation/source/developer-information/uml/rule_class.puml b/documentation/source/developer-information/uml/rule_class.puml new file mode 100644 index 0000000..0a24fc5 --- /dev/null +++ b/documentation/source/developer-information/uml/rule_class.puml @@ -0,0 +1,51 @@ +skinparam class { + BackgroundColor<> AlliceBlue + BorderColor<> LightSkyBlue +} + +abstract class rule.Rule { + +{abstract}examine(subject: source.SourceTree): issue.Issue<> + } + +class rule.FortranCharacterset { + +examine( subject: source.SourceTree ): issue.Issue<> +} +rule.Rule <|-- rule.FortranCharacterset + +class rule.TrailingWhitespace { + +examine( subject: source.SourceTree ): issue.Issue<> +} +rule.Rule ^-- rule.TrailingWhitespace + +abstract class rule.FortranRule { + +examine(subject: source.SourceTree): issue.Issue<> + +{abstract}examine_fortran(subject: source.FortranSource): issue.Issue<> + } +rule.Rule <|-- rule.FortranRule + +class rule.MissingVisibility <> { + +examine_fortran(subject: source.FortranSource): issue.Issue<> + } +rule.FortranRule <|-- rule.MissingVisibility + +class rule.MissingImplicit { + +<>__init__(require_everywhere: Bool = False) + +examine_fortran(subject: source.FortranSource): issue.Issue<> + } +rule.FortranRule <|-- rule.MissingImplicit + +class rule.MissingLegalNotice <> { + +examine_fortran(subject: source.FortranSource): issue.Issue<> + } +rule.FortranRule <|-- rule.MissingLegalNotice + +class rule.CommaSpace <> { + +examine_fortran(subject: source.FortranSource): issue.Issue<> +} +rule.FortranRule <|-- rule.CommaSpace + +abstract class rule.CRule <> { + +examine( subject: SourceTree ): Issue<> + +{abstract}examine_c(subject: source.CSource): issue.Issue + } +rule.Rule <|-- rule.CRule diff --git a/documentation/source/developer-information/uml/sequence_diagram.puml b/documentation/source/developer-information/uml/sequence_diagram.puml new file mode 100644 index 0000000..7a67bf5 --- /dev/null +++ b/documentation/source/developer-information/uml/sequence_diagram.puml @@ -0,0 +1,25 @@ +@startuml Checker Sequence Diagram +participant UserInterface + +UserInterface -> style.LFRicStyle : <> +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 diff --git a/documentation/source/developer-information/uml/source_class.puml b/documentation/source/developer-information/uml/source_class.puml new file mode 100644 index 0000000..c1dd6cf --- /dev/null +++ b/documentation/source/developer-information/uml/source_class.puml @@ -0,0 +1,88 @@ +skinparam class { + BackgroundColor<> AlliceBlue + BorderColor<> LightSkyBlue +} + + +abstract class source.SourceText { + +{abstract}get_text(): String +} + + +source.TextProcessor *-- source.SourceText +abstract class source.TextProcessor { + +<>__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 { + +<>__init__(source_file: Path|TextIO) + +get_text(): String +} +source.SourceText <|-- source.FileReader + + +class source.StringReader { + +<>__init__(source_string: String) + +get_text(): String +} +source.SourceText <|-- source.StringReader + + +source.SourceTree *-- source.SourceText +abstract class source.SourceTree { + +<>__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<> + +find_all(find_node: fparser.Fortran2003.Base<>, root: fparser.Fortran2003.Base): fparser.Fortran2003.Base<> + +{static}print_tree(root: fparser.Fortran2003.Base, indent: Integer = 0) + } +source.SourceTree <|-- source.FortranSource + + +class source.CSource <> { + +get_tree() + +get_tree_error() + } +source.SourceTree <|-- source.CSource + + +class source.PlainText { + +{static}get_name(): String + +get_tree(): String<> + +get_tree_error(): String<> +} +source.SourceTree <|-- source.PlainText diff --git a/documentation/source/developer-information/uml/style_class.puml b/documentation/source/developer-information/uml/style_class.puml new file mode 100644 index 0000000..51372ef --- /dev/null +++ b/documentation/source/developer-information/uml/style_class.puml @@ -0,0 +1,30 @@ +skinparam class { + BackgroundColor<> AlliceBlue + BorderColor<> LightSkyBlue +} + +rule.Rule "*" --o style.Style +abstract class style.Style { + +<>__init__( rules: Rule<> ) + +<>name(): String + +<>name(new: String) + +list_rules(): Rule<> + +check( source: SourceTree ): issue.Issue<> + } + +class LFRicStyle { + +__init__() + } +style.Style <|-- LFRicStyle + +class UMStyle <> { + +__init__() + } +style.Style <|-- UMStyle + +class style.BespokeStyle <> { + +__init__() + +addRule( rule: Rule ) + +removeRule( rule: Rule ) + } +style.Style <|-- style.BespokeStyle diff --git a/documentation/uml/CheckerDesign.puml b/documentation/uml/CheckerDesign.puml deleted file mode 100644 index 001f3e5..0000000 --- a/documentation/uml/CheckerDesign.puml +++ /dev/null @@ -1,197 +0,0 @@ -@startuml Checker Class Diagram - -skinparam class { - BackgroundColor<> AlliceBlue - BorderColor<> LightSkyBlue -} - -'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' -' inspector.source module - -class source.ProcessChain { - +textChain: List <>>> - +parser: SourceTree -} - -class source.Factory { - -{static}extension_map: Map <> - +{static}read_file(filename: String): source.SourceTree - +{static}read_file(file: File): source.SourceTree - +{static}add_extension_mapping(extension: String, parser: SourceTree<>, processors[]: SourceText<>) -} - - -abstract class source.SourceText { - +{abstract}get_text(): String -} - -abstract class source.TextProcessor { - -processor: TextProcessor - +<>__init__(source: SourceText) -} -source.SourceText <|-- source.TextProcessor -source.TextProcessor o- source.SourceText - -class source.FileReader { - +get_text(): String -} -source.SourceText <|-- source.FileReader - -class source.CPreProcessor { - +get_text(): String -} -source.TextProcessor <|-- source.CPreProcessor - -class source.FortranPreProcessor { - +get_text(): String -} -source.TextProcessor <|-- source.FortranPreProcessor - -class source.pFUnitProcessor { - +get_text(): String -} -source.TextProcessor <|-- source.pFUnitProcessor - - -abstract class source.SourceTree { - -text: SourceText - +<>__init__( text: SourceText ) - +get_text(): String - +{abstract}get_tree() - } - -class source.FortranTree { - -tree: fparser.Fortran2003.Program - +get_tree() - +{static}path( start: fparser.Fortran2003.*, path: String ): fparser.Fortran2003.* - +get_default_extensions() String <> - } -source.SourceTree <|-- source.FortranTree - -class source.CeeTree { - -tree: pycparse.Whatever - +get_tree() - } -source.SourceTree <|-- source.CeeTree - -'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' -' inspector.rule module - -abstract class rule.Rule { - +{abstract}examine( subject: SourceTree ): Issue<> - } - -abstract class rule.FortranRule { - +{abstract}examine( subject: FortranTree ): Issue<> - } -rule.Rule <|-- rule.FortranRule - -class rule.MissingVisibility <> { - +__init__( default: Visibility ) - } -rule.FortranRule <|-- rule.MissingVisibility - -class rule.MissingImplicit { - +__init__( default: Implicit ) - } -rule.FortranRule <|-- rule.MissingImplicit - -class rule.MissingLegalNotice <> { - +__init__( notice: String ) - } -rule.FortranRule <|-- rule.MissingLegalNotice - -class rule.FortranCharacterset { - __init__() -} -rule.FortranRule <|-- rule.FortranCharacterset - -class rule.CommaSpace <> { - __init__() -} -rule.FortranRule <|-- rule.CommaSpace - -class rule.PointersNullOnDeclaration <> { - __init__() - } -rule.FortranRule <|-- rule.PointersNullOnDeclaration - -abstract class rule.CeeRule { - +{abstract}examine( subject: CeeTree ): Issue<> - } -rule.Rule <|-- rule.CeeRule - -'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' -' inspector.style module - -abstract class style.Style { - -rules: Rule<> - +__init__( rules: Rule<> ) - +check( source: Source ): Issue<> - } -rule.Rule "+" -o style.Style - -class style.LFRicStyle { - +__init__() - } -style.Style <|-- style.LFRicStyle - -class style.UMStyle <> { - +__init__() - } -style.Style <|-- style.UMStyle - -class style.BespokeStyle <> { - +__init__() - +addRule( rule: Rule ) - +removeRule( rule: Rule ) - } -style.Style <|-- style.BespokeStyle - -'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' -' inspector.engine module - -class engine.CheckEngine { - -styles: Style<> - +__init__( styles: Style<>) - +check( filename: String <> ): Issue<> - } -style.Style "+" -o engine.CheckEngine - -'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' -' inspector.issue module - -class issue.Issue { - -description: String - +__init__( description: String ) - +__str__(): String - } - -@enduml - - -@startuml Checker Sequence Diagram -participant UserInterface - -UserInterface -> style.LFRicStyle : <> -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 diff --git a/pyproject.toml b/pyproject.toml index a8ffbc0..943d8a9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,8 @@ test = ['pytest', 'pytest-cov', 'mypy'] performance = ['pytest', 'pytest-benchmark', 'matplotlib'] docs = ['sphinx < 7.0.0', 'sphinx-autodoc-typehints', + 'sphinxcontrib-plantuml>=0.30.0', + 'pillow>=11.0.0', 'pydata-sphinx-theme>=0.15.2'] release = ['setuptools', 'wheel', 'twine']