From 6329d9a809123361733b5ffa669fff605052bed4 Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Thu, 17 Oct 2024 09:19:57 +0100 Subject: [PATCH 1/4] 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'] From 2e0cd00087257af67d4943c4c8cd1833f7ba16d2 Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Fri, 15 Nov 2024 09:04:46 +0000 Subject: [PATCH 2/4] Improved diagrams. --- .../source/developer-information/design.rst | 9 ++++- .../uml/engine_class.puml | 4 +- .../developer-information/uml/rule_class.puml | 8 +--- .../uml/sequence_diagram.puml | 40 +++++++++++++------ .../uml/source_class.puml | 40 +++++++++---------- .../uml/style_class.puml | 5 ++- 6 files changed, 65 insertions(+), 41 deletions(-) diff --git a/documentation/source/developer-information/design.rst b/documentation/source/developer-information/design.rst index 906288b..bbb21e0 100644 --- a/documentation/source/developer-information/design.rst +++ b/documentation/source/developer-information/design.rst @@ -15,6 +15,7 @@ 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. @@ -24,14 +25,20 @@ abstract syntax tree, gained by parsing the source. :align: center :width: 100% -Operation is orchestrated through the `Engine` class. It makes use of a +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% diff --git a/documentation/source/developer-information/uml/engine_class.puml b/documentation/source/developer-information/uml/engine_class.puml index 300b385..c888dd9 100644 --- a/documentation/source/developer-information/uml/engine_class.puml +++ b/documentation/source/developer-information/uml/engine_class.puml @@ -28,6 +28,8 @@ style.Style "+" -o engine.CheckEngine class issue.Issue { -description: String - +__init__( description: String ) + -filename: Path <> + -line: Integer <> + +__init__( description: String, line: Integer <>, filename: Path <> ) +__str__(): String } diff --git a/documentation/source/developer-information/uml/rule_class.puml b/documentation/source/developer-information/uml/rule_class.puml index 0a24fc5..8f72bb6 100644 --- a/documentation/source/developer-information/uml/rule_class.puml +++ b/documentation/source/developer-information/uml/rule_class.puml @@ -2,6 +2,7 @@ skinparam class { BackgroundColor<> AlliceBlue BorderColor<> LightSkyBlue } +skinparam groupInheritance 2 abstract class rule.Rule { +{abstract}examine(subject: source.SourceTree): issue.Issue<> @@ -21,7 +22,7 @@ abstract class rule.FortranRule { +examine(subject: source.SourceTree): issue.Issue<> +{abstract}examine_fortran(subject: source.FortranSource): issue.Issue<> } -rule.Rule <|-- rule.FortranRule +rule.Rule <|--- rule.FortranRule class rule.MissingVisibility <> { +examine_fortran(subject: source.FortranSource): issue.Issue<> @@ -39,11 +40,6 @@ class rule.MissingLegalNotice <> { } 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 diff --git a/documentation/source/developer-information/uml/sequence_diagram.puml b/documentation/source/developer-information/uml/sequence_diagram.puml index 7a67bf5..2c6e28a 100644 --- a/documentation/source/developer-information/uml/sequence_diagram.puml +++ b/documentation/source/developer-information/uml/sequence_diagram.puml @@ -1,25 +1,41 @@ @startuml Checker Sequence Diagram participant UserInterface +participant "missing_implicit:rule.MissingImplicit" as MissingImplicit +participant "my_style:rule.LFRicStyle" as LFRicStyle +participant engine.CheckEngine as CheckEngine +participant source.SourceFactory +participant "some_text:source.SourceFileReader" as SourceFileReader +participant "source_tree:source.FortranSource" as FortranSource -UserInterface -> style.LFRicStyle : <> -activate style.LFRicStyle +UserInterface -\ MissingImplicit : <> +activate MissingImplicit -style.LFRicStyle -> rule.MissingImplicit : MissingImplicit(None) -activate rule.MissingImplicit +UserInterface -\ LFRicStyle : LFRicStyle(mising_implicit) <> +activate LFRicStyle -UserInterface -> engine.CheckEngine : CheckEngine(LFRicStyle) -activate engine.CheckEngine +UserInterface -\ CheckEngine : CheckEngine(my_style) <> +activate CheckEngine -UserInterface -> engine.CheckEngine : check(SourceFile) +UserInterface -> source.SourceFactory: read_file(filename) -engine.CheckEngine -> style.LFRicStyle : check(Program) +source.SourceFactory -\ SourceFileReader: SourceFileReader(filename) <> +activate SourceFileReader -style.LFRicStyle -> rule.MissingImplicit : examine(Program) +source.SourceFactory -\ FortranSource: FortranSource(some_text) <> +activate FortranSource -rule.MissingImplicit --> style.Style : Issues[] +source.SourceFactory --> UserInterface: source_tree -style.Style --> engine.CheckEngine : Issues[] +UserInterface -> CheckEngine : check(source_tree) -engine.CheckEngine --> UserInterface : Issues[] +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 diff --git a/documentation/source/developer-information/uml/source_class.puml b/documentation/source/developer-information/uml/source_class.puml index c1dd6cf..1ae9dca 100644 --- a/documentation/source/developer-information/uml/source_class.puml +++ b/documentation/source/developer-information/uml/source_class.puml @@ -4,62 +4,62 @@ skinparam class { } -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 { +class source.CPreProcessor <> { +{static}get_name(): String +get_text(): String } source.TextProcessor <|-- source.CPreProcessor -class source.FortranPreProcessor { +class source.FortranPreProcessor <> { +{static}get_name(): String +get_text(): String } source.TextProcessor <|-- source.FortranPreProcessor -class source.pFUnitProcessor { +class source.pFUnitProcessor <> { +{static}get_name(): String +get_text(): String } source.TextProcessor <|-- source.pFUnitProcessor -class source.FileReader { +abstract class source.TextProcessor <> { + +<>__init__(source: SourceText) + +{abstract}{static}get_name(): String + +{abstract}get_text(): String +} +source.TextProcessor *-- source.SourceText + + +class source.FileReader <> { +<>__init__(source_file: Path|TextIO) +get_text(): String } source.SourceText <|-- source.FileReader -class source.StringReader { +class source.StringReader <> { +<>__init__(source_string: String) +get_text(): String } source.SourceText <|-- source.StringReader -source.SourceTree *-- source.SourceText +abstract class source.SourceText <> { + +{abstract}get_text(): String +} +source.SourceText <|-- source.TextProcessor + + abstract class source.SourceTree { +<>__init__( text: SourceText ) +{abstract}get_tree() +{abstract}get_tree_error(): String +get_text(): String } +source.SourceTree *---- source.SourceText class source.FortranSource { diff --git a/documentation/source/developer-information/uml/style_class.puml b/documentation/source/developer-information/uml/style_class.puml index 51372ef..c5bdc1f 100644 --- a/documentation/source/developer-information/uml/style_class.puml +++ b/documentation/source/developer-information/uml/style_class.puml @@ -2,8 +2,11 @@ skinparam class { BackgroundColor<> AlliceBlue BorderColor<> LightSkyBlue } +skinparam groupInheritance 2 -rule.Rule "*" --o style.Style +left to right direction + +rule.Rule "*" -o style.Style abstract class style.Style { +<>__init__( rules: Rule<> ) +<>name(): String From 12465d64e6fff54e3fe039ee47830afb0e180a89 Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Fri, 15 Nov 2024 11:25:03 +0000 Subject: [PATCH 3/4] Thoughts on future design. --- .../source/developer-information/design.rst | 2 + .../source/developer-information/future.rst | 19 +++- .../uml/future_class_diagram.puml | 91 ------------------- .../uml/future_sequence_diagram.puml | 40 +++++--- .../uml/sequence_diagram.puml | 22 +++-- 5 files changed, 56 insertions(+), 118 deletions(-) delete mode 100644 documentation/source/developer-information/uml/future_class_diagram.puml diff --git a/documentation/source/developer-information/design.rst b/documentation/source/developer-information/design.rst index bbb21e0..d547960 100644 --- a/documentation/source/developer-information/design.rst +++ b/documentation/source/developer-information/design.rst @@ -1,3 +1,5 @@ +.. _design-page: + Design ====== diff --git a/documentation/source/developer-information/future.rst b/documentation/source/developer-information/future.rst index 3901fbc..ea445c0 100644 --- a/documentation/source/developer-information/future.rst +++ b/documentation/source/developer-information/future.rst @@ -23,19 +23,28 @@ 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`. +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 -A proposed design is given here: +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. -.. uml:: uml/future_class_diagram.puml - :caption: UML class diagram of potential coercive implementation. +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% diff --git a/documentation/source/developer-information/uml/future_class_diagram.puml b/documentation/source/developer-information/uml/future_class_diagram.puml deleted file mode 100644 index a0d15bb..0000000 --- a/documentation/source/developer-information/uml/future_class_diagram.puml +++ /dev/null @@ -1,91 +0,0 @@ -@startuml Styler Class Diagram - -skinparam class { - BackgroundColor<> AlliceBlue - BorderColor<> LightSkyBlue -} - -' inspector.rule module - -abstract class rule.Rule { - } - -abstract class rule.TreeRule { - +{abstract}examine( program: fParser.Program ): Issue<> - } - -rule.Rule <|-- rule.TreeRule - -class rule.MissingImplicit { - +__init__( default: Implicit ) - } -rule.TreeRule <|-- rule.MissingImplicit - -class rule.MissingLegalNotice <> { - +__init__( notice: String ) - } -rule.TreeRule <|-- rule.MissingLegalNotice - -class rule.MissingVisibility <> { - +__init__( default: Visibility ) - } -rule.TreeRule <|-- rule.MissingVisibility - -abstract class rule.PatternRule { - +{abstract}examine( source: String ): Issue<> - } -rule.Rule <|-- rule.PatternRule - -class rule.FortranCharacterset <> { - } -rule.PatternRule <|-- rule.FortranCharacterset - -class rule.IndentSize <>{ - +__init__( width: Integer ) - } -rule.PatternRule <|-- rule.IndentSize - -class rule.CommaSpace <> { - } -rule.PatternRule <|-- rule.CommaSpace - - -' inspector.style module - -abstract class style.Style { - -patternRules: PatternRule<> - -treeRules: TreeRul<> - +__init__( rules: Rule<> ) - +check( program: fParser.Program ): Issue<> - } -rule.Rule "+"- style.Style - -class style.LFRicStyle { - +def __init__() - } -style.Style <|-- style.LFRicStyle - -class style.BespokeStyle <> { - +def __init__() - +addRule( rule: Rule ) - +removeRule( rule: Rule ) - } -style.Style <|-- style.BespokeStyle - - -class engine.CheckEngine { - -styles: Style<> - +__init__( styles: Style<>) - +check( source: File ): Issue<> - } -style.Style "+"- engine.CheckEngine - - -' inspector.issue module - -class issue.Issue { - +__init__( description: String ) - +getDescription() - } - -@enduml diff --git a/documentation/source/developer-information/uml/future_sequence_diagram.puml b/documentation/source/developer-information/uml/future_sequence_diagram.puml index a8404a9..e9b31f7 100644 --- a/documentation/source/developer-information/uml/future_sequence_diagram.puml +++ b/documentation/source/developer-information/uml/future_sequence_diagram.puml @@ -1,26 +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 -UserInterface -> style.LFRicStyle : <> -activate style.LFRicStyle +activate SourceFactory -style.LFRicStyle -> rule.MissingImplicit : MissingImplicit(None) -activate rule.MissingImplicit +UserInterface -\ LFRicStyle : <> +activate LFRicStyle -UserInterface -> engine.CheckEngine : CheckEngine(LFRicStyle) -activate engine.CheckEngine +LFRicStyle -\ MissingImplicit : MissingImplicit(None) <> +activate MissingImplicit -UserInterface -> engine.CheckEngine : check(SourceFile) +UserInterface -\ CheckEngine : CheckEngine(my_style) +activate CheckEngine -engine.CheckEngine -> style.LFRicStyle : check(Program) +UserInterface -> SourceFactory : SourceFactory(filename) +SourceFactory -\ FortranSource : <> +activate FortranSource +SourceFactory --> UserInterface : fortran_source -style.LFRicStyle -> style.MissingImplicit : examine(Program) +UserInterface -> CheckEngine : check(fortran_source) -style.MissingImplicit --> style.Style : Issues[] +CheckEngine -> LFRicStyle : check(fortran_source) -style.Style --> engine.CheckEngine : Issues[] +LFRicStyle -> MissingImplicit : examine(fortran_source) -engine.CheckEngine --> UserInterface : Issues[] +alt "Implicit statement is missing" +MissingImplicit -\ FortranSource : insert_statement(Implicit(None)) +end + +MissingImplicit --> LFRicStyle : Issue <> + +LFRicStyle --> CheckEngine : Issue <> + +CheckEngine --> UserInterface : Issue <> @enduml diff --git a/documentation/source/developer-information/uml/sequence_diagram.puml b/documentation/source/developer-information/uml/sequence_diagram.puml index 2c6e28a..031a7d3 100644 --- a/documentation/source/developer-information/uml/sequence_diagram.puml +++ b/documentation/source/developer-information/uml/sequence_diagram.puml @@ -1,30 +1,32 @@ @startuml Checker Sequence Diagram participant UserInterface -participant "missing_implicit:rule.MissingImplicit" as MissingImplicit -participant "my_style:rule.LFRicStyle" as LFRicStyle participant engine.CheckEngine as CheckEngine -participant source.SourceFactory +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 -UserInterface -\ MissingImplicit : <> -activate MissingImplicit +activate SourceFactory -UserInterface -\ LFRicStyle : LFRicStyle(mising_implicit) <> +UserInterface -\ LFRicStyle : LFRicStyle() <> activate LFRicStyle +LFRicStyle -\ MissingImplicit : <> +activate MissingImplicit + UserInterface -\ CheckEngine : CheckEngine(my_style) <> activate CheckEngine -UserInterface -> source.SourceFactory: read_file(filename) +UserInterface -> SourceFactory: read_file(filename) -source.SourceFactory -\ SourceFileReader: SourceFileReader(filename) <> +SourceFactory -\ SourceFileReader: SourceFileReader(filename) <> activate SourceFileReader -source.SourceFactory -\ FortranSource: FortranSource(some_text) <> +SourceFactory -\ FortranSource: FortranSource(some_text) <> activate FortranSource -source.SourceFactory --> UserInterface: source_tree +SourceFactory --> UserInterface: source_tree UserInterface -> CheckEngine : check(source_tree) From 717c24ba59c368551f5f4333d92f30668e099da2 Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Fri, 15 Nov 2024 11:48:28 +0000 Subject: [PATCH 4/4] Note PlantUML dependency. --- README.rst | 27 +++++++++++++++++++++++++++ pyproject.toml | 3 ++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index d15c363..ee7544b 100644 --- a/README.rst +++ b/README.rst @@ -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 ~~~~~ diff --git a/pyproject.toml b/pyproject.toml index 943d8a9..734bbdb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,7 +27,8 @@ docs = ['sphinx < 7.0.0', 'sphinx-autodoc-typehints', 'sphinxcontrib-plantuml>=0.30.0', 'pillow>=11.0.0', - 'pydata-sphinx-theme>=0.15.2'] + 'pydata-sphinx-theme>=0.15.2', + 'sphinxcontrib.plantuml'] release = ['setuptools', 'wheel', 'twine'] [project.scripts]