From 0340b1c76574e068fd724cac3890cdf5a141493b Mon Sep 17 00:00:00 2001 From: MatthewHambley Date: Thu, 24 Aug 2023 12:58:49 +0100 Subject: [PATCH 01/10] Don't try to validate length of strings without length (#118) Single character appear like character array variables but with no length. The variable length array style rule should not apply and this change adds an exemption for them. --- source/stylist/fortran.py | 8 +++- .../fortran_auto_char_array_intent_test.py | 45 ++++++++++++++++--- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/source/stylist/fortran.py b/source/stylist/fortran.py index 2cb0741..6fd4512 100644 --- a/source/stylist/fortran.py +++ b/source/stylist/fortran.py @@ -601,7 +601,8 @@ class AutoCharArrayIntent(FortranRule): subroutine or function arguments have intent(in) to avoid writing outside the given array. """ - def _message(self, name, intent): + @staticmethod + def __message(name, intent): return (f"Arguments of type character(*) must have intent IN, but " f"{name} has intent {intent}.") @@ -630,6 +631,9 @@ def examine_fortran(self, subject: FortranSource) -> List[Issue]: if not type_spec.items[0] == "CHARACTER": continue param_value = type_spec.items[1] + # If no length is specified we don't care + if param_value is None: + continue # This might be a length selector, if so get the param value if isinstance(param_value, Fortran2003.Length_Selector): param_value = param_value.items[1] @@ -654,7 +658,7 @@ def examine_fortran(self, subject: FortranSource) -> List[Issue]: if intent_attr.items[1].string == "IN": continue issues.append(Issue( - self._message( + self.__message( declaration.items[2].string, intent_attr.items[1] ), diff --git a/unit-tests/fortran_auto_char_array_intent_test.py b/unit-tests/fortran_auto_char_array_intent_test.py index 630d7e7..d823128 100644 --- a/unit-tests/fortran_auto_char_array_intent_test.py +++ b/unit-tests/fortran_auto_char_array_intent_test.py @@ -5,7 +5,7 @@ import stylist.fortran from stylist.source import FortranSource, SourceStringReader -TEST_CASE = """ +NORMAL_TEST_CASE = """ program cases ! A char array outside a function or subroutine, no exception character (*) :: autochar_glob @@ -30,13 +30,36 @@ end program cases """ -TEST_EXPECTATION = [ +NORMAL_TEST_EXPECTATION = [ ('12: Arguments of type character(*) must have intent IN, ' 'but autochar_inout has intent INOUT.'), ('14: Arguments of type character(*) must have intent IN, ' 'but autochar_out has intent OUT.') ] +SINGLE_CHAR_CASE = """ +program single_char + + implicit none + + character :: single_program_character + +contains + + subroutine single_character( char_in, char_inout, char_out, char_def ) + + implicit none + + character, intent(in) :: char_in + character, intent(inout) :: char_inout + character, intent(out) :: char_out + character :: char_def + + end subroutine single_character + +end program single_char +""" + class TestAutoCharArrayIntent: """ @@ -44,14 +67,26 @@ class TestAutoCharArrayIntent: have intent(in) """ - def test(self): + def test_normal_behaviour(self): """ Ensures the test case produces exactly the issues in expectation """ - reader = SourceStringReader(TEST_CASE) + reader = SourceStringReader(NORMAL_TEST_CASE) source = FortranSource(reader) unit_under_test = stylist.fortran.AutoCharArrayIntent() issues = unit_under_test.examine(source) strings = [str(issue) for issue in issues] - assert strings == TEST_EXPECTATION + assert strings == NORMAL_TEST_EXPECTATION + + def test_single_char(self): + """ + Ensures the test can handle no length being specified. + """ + reader = SourceStringReader(SINGLE_CHAR_CASE) + source = FortranSource(reader) + test_unit = stylist.fortran.AutoCharArrayIntent() + issues = test_unit.examine(source) + + strings = [str(issue) for issue in issues] + assert strings == [] From c01def9b5eeb2885e45b5097ea0bbf3297da2eed Mon Sep 17 00:00:00 2001 From: MatthewHambley Date: Fri, 25 Aug 2023 09:51:39 +0100 Subject: [PATCH 02/10] Update documentation index on release (#124) This adds a workflow which automatically updates an unordered list in index.html in the documentation branch with new tagged releases when they are pushed. --- .github/bin/update-index | 46 ++++++++++++++++++++++++++++++++++++ .github/workflows/sphinx.yml | 2 ++ 2 files changed, 48 insertions(+) create mode 100755 .github/bin/update-index diff --git a/.github/bin/update-index b/.github/bin/update-index new file mode 100755 index 0000000..7bf2035 --- /dev/null +++ b/.github/bin/update-index @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +############################################################################## +# Add a new release's documentation to the documentation index page. +# +# Usage: +# update-index +############################################################################## +from argparse import ArgumentParser +from pathlib import Path +from xml.etree import ElementTree + +if __name__ == '__main__': + cli_parser = ArgumentParser(description="Add a new release to the index") + cli_parser.add_argument('index_file', type=Path, + help="Filename of index file.") + cli_parser.add_argument('release', + help="Release tag name") + arguments = cli_parser.parse_args() + + ns = {'html': 'http://www.w3.org/1999/xhtml'} + + ElementTree.register_namespace('', 'http://www.w3.org/1999/xhtml') + index_doc = ElementTree.parse(arguments.index_file) + + release_list = index_doc.find('.//html:ul[@id="releases"]', namespaces=ns) + if release_list is None: + raise Exception("Unable to find release list") + + last_child = list(release_list)[-1] + item_indent = release_list.text + list_indent = last_child.tail + last_child.tail = item_indent + + new_release = ElementTree.SubElement(release_list, 'li') + new_release.tail = list_indent + new_anchor = ElementTree.SubElement(new_release, 'a') + new_anchor.attrib['href'] = arguments.release + new_anchor.text = arguments.release + + document_text = ElementTree.tostring(index_doc.getroot(), encoding='unicode') + with arguments.index_file.open('w') as fhandle: + # Doctype is not preserved by the parser so we need to recreate it. + # + print('''''', file=fhandle) + print(document_text, file=fhandle) diff --git a/.github/workflows/sphinx.yml b/.github/workflows/sphinx.yml index 9cc022e..ce0a47e 100644 --- a/.github/workflows/sphinx.yml +++ b/.github/workflows/sphinx.yml @@ -97,6 +97,8 @@ jobs: set -x rsync -a stylist/documentation/build/html/ gh-pages/${{github.ref_name}} git -C gh-pages add ${{github.ref_name}} + stylist/.github/bin/update-index gh-pages/index.html ${{github.ref_name}} + git -C gh-pages add index.html - name: Commit documentation if: github.event_name == 'push' From 8fdcf9f896713c6d9c4994e18952ff24f6ee276e Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Tue, 26 Sep 2023 16:59:26 +0100 Subject: [PATCH 03/10] Improve naked literal handling of array indeces (#127) * Replace `setup.py` script with `stylist.toml` configuration file. * Turned out the `.toml` file wasn't being used. * Discovered mistake in pull request workflow. * Since `setup.py` has been removed we can't check it with `mypy` * Added Conda Forge badge to ReadMe. * Constrain fParser version. * Remove reference to setup.py from documentation. * Added test for fault condition. * Implement solution. * Style issue. * Problem with array indexing. * Reworked for improved working. * Fixed documentation. * Nail Sphinx version to get around problem with Read-t-docs theme --- pyproject.toml | 2 +- source/stylist/fortran.py | 37 +++++++---- source/stylist/style.py | 2 +- unit-tests/fortran_naked_literal_test.py | 79 ++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 14 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a58a797..550d836 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dynamic = ['version'] dev = ['check-manifest', 'flake8'] test = ['pytest', 'pytest-cov', 'mypy'] performance = ['pytest', 'pytest-benchmark', 'matplotlib'] -docs = ['sphinx', +docs = ['sphinx < 7.0.0', 'sphinx-autodoc-typehints', 'sphinx-rtd-theme>=1.2.2'] release = ['setuptools', 'wheel', 'twine'] diff --git a/source/stylist/fortran.py b/source/stylist/fortran.py index 6fd4512..8cfb1b3 100644 --- a/source/stylist/fortran.py +++ b/source/stylist/fortran.py @@ -690,22 +690,35 @@ def examine_fortran(self, subject: FortranSource) -> List[Issue]: candidates.extend(fp_walk(subject.get_tree(), Fortran2003.Real_Literal_Constant)) - for constant in candidates: - if constant.items[1] is None: - if isinstance(constant.parent, Fortran2003.Assignment_Stmt): - name = str(fp_get_child(constant.parent, Fortran2003.Name)) - message = f'Literal value assigned to "{name}"' \ - ' without kind' - elif isinstance(constant.parent.parent, - (Fortran2003.Entity_Decl, - Fortran2003.Component_Decl)): - name = str(fp_get_child(constant.parent.parent, + for literal in candidates: + if literal.items[1] is not None: # Skip when kind is present + continue + + name: Optional[str] = None + parent = literal.parent + while parent is not None: + if isinstance(parent, Fortran2003.Part_Ref): + name = str(fp_get_child(parent, Fortran2003.Name)) + message = f'Literal value index used with "{name}"' \ + ' without kind' + break + elif isinstance(parent, (Fortran2003.Assignment_Stmt, + Fortran2003.Entity_Decl, + Fortran2003.Component_Decl)): + array_slice = fp_get_child(parent, Fortran2003.Part_Ref) + if array_slice is None: + name = str(fp_get_child(parent, Fortran2003.Name)) + else: + name = str(fp_get_child(array_slice, Fortran2003.Name)) message = f'Literal value assigned to "{name}"' \ ' without kind' + break else: - message = 'Literal value without "kind"' - issues.append(Issue(message, line=_line(constant))) + parent = parent.parent + if name is None: + message = 'Literal value without "kind"' + issues.append(Issue(message, line=_line(literal))) return issues diff --git a/source/stylist/style.py b/source/stylist/style.py index 6fc3e81..5a4f3c3 100644 --- a/source/stylist/style.py +++ b/source/stylist/style.py @@ -24,7 +24,7 @@ class Style(ABC): def __init__(self, *rules: Rule) -> None: """ - :param *args: Rules which make up this style. + :param rules: Rules which make up this style. """ self.__rules = list(rules) self.__name = f"Unnamed style {self.__unnamed_tally}" diff --git a/unit-tests/fortran_naked_literal_test.py b/unit-tests/fortran_naked_literal_test.py index 7652f22..6ae8926 100644 --- a/unit-tests/fortran_naked_literal_test.py +++ b/unit-tests/fortran_naked_literal_test.py @@ -259,3 +259,82 @@ def test_float(self): test_unit = NakedLiteral(integers=False) issues = test_unit.examine(source) assert sorted([str(issue) for issue in issues]) == self._expected_float + + __array_text = dedent(''' + module array_mod + use iso_fortran_env, only : int64, int32, real64, real32 + implicit none + integer(int32) :: integer_32_good(2_int32), integer_32_bad(2) + integer(kind=int64) :: integer_64_good(2_int64), integer_64_bad(2) + real(real32) :: real_32_good(2_int32), real_32_bad(2) + real(kind=real64) :: real_64_good(2_int64), real_64_bad(2) + contains + subroutine assign_values() + implicit none + integer_32_good = (/12_int32, 24_int32/) + integer_32_bad = (/13, 26/) + integer_64_good = (/14_int64, 28_int64/) + integer_64_bad = (/15, 30/) + real_32_good = (/2.3_real32, 4.6_real32/) + real_32_bad = (/3.4, 6.8/) + real_64_good = (/4.5_real64, 6.0_real64/) + real_64_bad = (/5.6, 11.2/) + end subroutine assign_values + subroutine assign_elements() + integer_32_good(1_int32) = 4_int32 + integer_32_bad(2) = 5 + integer_64_good(1_int64) = 6_int64 + integer_64_bad(2) = 7 + real_32_good(1_int32) = 8.2_real32 + real_32_bad(2) = 16.4 + real_64_good(1_int64) = 9.3_real64 + real_64_bad(2) = 18.6 + end subroutine + subroutine array_slices() + integer_32_bad(1:1) = 20_int32 + integer_64_bad(1:1) = 21_int64 + real_32_bad(1:1) = 22.2_real32 + real_64_bad(1:1) = 23.3_real64 + end subroutine + end module array_mod + ''') + + __expected_array = [ + '13: Literal value assigned to "integer_32_bad" without kind', + '13: Literal value assigned to "integer_32_bad" without kind', + '15: Literal value assigned to "integer_64_bad" without kind', + '15: Literal value assigned to "integer_64_bad" without kind', + '17: Literal value assigned to "real_32_bad" without kind', + '17: Literal value assigned to "real_32_bad" without kind', + '19: Literal value assigned to "real_64_bad" without kind', + '19: Literal value assigned to "real_64_bad" without kind', + '23: Literal value assigned to "integer_32_bad" without kind', + '23: Literal value index used with "integer_32_bad" without kind', + '25: Literal value assigned to "integer_64_bad" without kind', + '25: Literal value index used with "integer_64_bad" without kind', + '27: Literal value assigned to "real_32_bad" without kind', + '27: Literal value index used with "real_32_bad" without kind', + '29: Literal value assigned to "real_64_bad" without kind', + '29: Literal value index used with "real_64_bad" without kind', + '32: Literal value index used with "integer_32_bad" without kind', + '32: Literal value index used with "integer_32_bad" without kind', + '33: Literal value index used with "integer_64_bad" without kind', + '33: Literal value index used with "integer_64_bad" without kind', + '34: Literal value index used with "real_32_bad" without kind', + '34: Literal value index used with "real_32_bad" without kind', + '35: Literal value index used with "real_64_bad" without kind', + '35: Literal value index used with "real_64_bad" without kind', + '5: Literal value assigned to "integer_32_bad" without kind', + '6: Literal value assigned to "integer_64_bad" without kind', + '7: Literal value assigned to "real_32_bad" without kind', + '8: Literal value assigned to "real_64_bad" without kind' + ] + + def test_arrays(self): + reader = SourceStringReader(self.__array_text) + source = FortranSource(reader) + + test_unit = NakedLiteral(integers=True, reals=True) + issues = test_unit.examine(source) + assert sorted([str(issue) for issue in issues]) \ + == self.__expected_array From 951a9a819c49fbb1cd96feaa1dc79c50fbd8b11c Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Mon, 27 Nov 2023 10:47:37 +0000 Subject: [PATCH 04/10] Don't baulk on intent(in) pointers (#120) * Replace `setup.py` script with `stylist.toml` configuration file. * Turned out the `.toml` file wasn't being used. * Discovered mistake in pull request workflow. * Since `setup.py` has been removed we can't check it with `mypy` * Added Conda Forge badge to ReadMe. * Constrain fParser version. * Remove reference to setup.py from documentation. * Pointer initialisation rules doesn't apply to abstract interfaces. * Or normal interfaces. --- source/stylist/fortran.py | 7 ++++++ unit-tests/fortran_pointer_init_test.py | 30 +++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/source/stylist/fortran.py b/source/stylist/fortran.py index 8cfb1b3..54e260c 100644 --- a/source/stylist/fortran.py +++ b/source/stylist/fortran.py @@ -393,6 +393,13 @@ def _data(root: Fortran2003.Base, attribute_specification): continue + # @todo This is quite ugly + # + potential_interface_block = data_declaration.parent.parent.parent + if isinstance(potential_interface_block, + Fortran2003.Interface_Block): + continue + for entity in fp_walk(data_declaration, entity_declaration): if str(fp_get_child(entity, Fortran2003.Name)) in ignore_names: continue diff --git a/unit-tests/fortran_pointer_init_test.py b/unit-tests/fortran_pointer_init_test.py index 2367362..1311fb4 100644 --- a/unit-tests/fortran_pointer_init_test.py +++ b/unit-tests/fortran_pointer_init_test.py @@ -166,8 +166,8 @@ def test_pointer_init(self, def test_arguments_without_intent(self): """ - Checks that the rule can cope with arguments which do not specify an - intent. + Checks that the rule can cope with arguments which should not specify + an intent. """ source_text = dedent(''' subroutine my_sub( first, second ) @@ -192,3 +192,29 @@ def test_arguments_without_intent(self): issue_descriptions = [str(issue) for issue in issues] assert issue_descriptions == [] + + def test_abstract_interface(self): + """ + Checks that the rule can cope with arguments which are intent in and + user defined types. + """ + source_text = dedent( + ''' + module this_mod + abstract interface + subroutine their_sub( first, second ) + implicit none + type(thang_type), intent(in), pointer :: first + class(thang_type), pointer, intent(in) :: second + end subroutine their_sub + end interface + end module this_mod + ''').strip() + source_reader = SourceStringReader(source_text) + source = FortranSource(source_reader) + + test_unit = stylist.fortran.MissingPointerInit() + issues = test_unit.examine(source) + + issue_descriptions = [str(issue) for issue in issues] + assert issue_descriptions == [] From 0ee4423057bb934581850e09b30f9245d1d3b83c Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Mon, 27 Nov 2023 10:50:54 +0000 Subject: [PATCH 05/10] Kind pattern rule to support absent kind (#121) * Replace `setup.py` script with `stylist.toml` configuration file. * Turned out the `.toml` file wasn't being used. * Discovered mistake in pull request workflow. * Since `setup.py` has been removed we can't check it with `mypy` * Added Conda Forge badge to ReadMe. * Constrain fParser version. * Remove reference to setup.py from documentation. * Allow pattern to be optional. * Style and typing --- source/stylist/fortran.py | 39 ++++++++++++++------ unit-tests/fortran_kind_pattern_test.py | 49 ++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 17 deletions(-) diff --git a/source/stylist/fortran.py b/source/stylist/fortran.py index 54e260c..4982032 100644 --- a/source/stylist/fortran.py +++ b/source/stylist/fortran.py @@ -8,9 +8,8 @@ """ import re from abc import ABC, abstractmethod -from collections import defaultdict from typing import (Container, Dict, List, Optional, Pattern, Sequence, Type, - Union) + Union, Any, cast) import fparser.two.Fortran2003 as Fortran2003 # type: ignore import fparser.two.Fortran2008 as Fortran2008 # type: ignore @@ -539,8 +538,8 @@ class KindPattern(FortranRule): " not fit the pattern /{pattern}/." def __init__(self, *, # There are no positional arguments. - integer: Union[str, Pattern], - real: Union[str, Pattern]): + integer: Optional[Union[str, Pattern]] = None, + real: Optional[Union[str, Pattern]] = None): """ Patterns are set only for integer and real data types however Fortran supports many more. Logical and Complex for example. For those cases a @@ -549,13 +548,16 @@ def __init__(self, *, # There are no positional arguments. :param integer: Regular expression which integer kinds must match. :param real: Regular expression which real kinds must match. """ - self._patterns: Dict[str, Pattern] \ - = defaultdict(lambda: re.compile(r'.*')) - if isinstance(integer, str): + self._patterns: Dict[str, Optional[Pattern]] = {'logical': None} + if integer is None: + pass + elif isinstance(integer, str): self._patterns['integer'] = re.compile(integer) else: self._patterns['integer'] = integer - if isinstance(real, str): + if real is None: + pass + elif isinstance(real, str): self._patterns['real'] = re.compile(real) else: self._patterns['real'] = real @@ -582,19 +584,34 @@ def examine_fortran(self, subject: FortranSource) -> List[Issue]: (Fortran2003.Data_Component_Def_Stmt, Fortran2003.Type_Declaration_Stmt)): type_spec: Fortran2003.Intrinsic_Type_Spec = candidate.items[0] + data_type: str = type_spec.items[0].lower() + + if self._patterns.get(data_type) is None: + continue + pattern = cast(Pattern[Any], self._patterns[data_type]) + kind_selector: Fortran2003.Kind_Selector = type_spec.items[1] + if kind_selector is None: + entity_declaration = candidate.items[2] + message = self._ISSUE_TEMPLATE.format( + type=data_type, + kind='', + name=entity_declaration, + pattern=pattern.pattern) + issues.append(Issue(message, + line=_line(candidate))) + continue if isinstance(kind_selector, Fortran2003.Kind_Selector): - data_type: str = type_spec.items[0].lower() kind: str = str(kind_selector.children[1]) - match = self._patterns[data_type].match(kind) + match = pattern.match(kind) if match is None: entity_declaration = candidate.items[2] message = self._ISSUE_TEMPLATE.format( type=data_type, kind=kind, name=entity_declaration, - pattern=self._patterns[data_type].pattern) + pattern=pattern.pattern) issues.append(Issue(message, line=_line(candidate))) diff --git a/unit-tests/fortran_kind_pattern_test.py b/unit-tests/fortran_kind_pattern_test.py index b0b4109..bd95c68 100644 --- a/unit-tests/fortran_kind_pattern_test.py +++ b/unit-tests/fortran_kind_pattern_test.py @@ -9,6 +9,8 @@ import re from textwrap import dedent +import pytest + from stylist.fortran import KindPattern from stylist.source import (FortranPreProcessor, FortranSource, @@ -238,8 +240,9 @@ def test_failing(self): assert len(issues) == 28 - def test_missing_kind(self): - case = dedent(""" + @pytest.fixture + def missing_kind_text(self) -> str: + return dedent(""" module passing_mod implicit none integer :: global_int @@ -273,9 +276,43 @@ def test_missing_kind(self): end module passing_mod """) - reader = SourceStringReader(case) + def test_missing_kind_expected(self, missing_kind_text): + reader = SourceStringReader(missing_kind_text) source = FortranSource(reader) - unit_under_test = KindPattern(integer=r'.+_beef', - real=re.compile(r'.+_cheese')) + unit_under_test = KindPattern() issues = unit_under_test.examine(source) - assert len(issues) == 0 + assert [str(issue) for issue in issues] == [] + + def test_missing_kind_integer(self, missing_kind_text): + reader = SourceStringReader(missing_kind_text) + source = FortranSource(reader) + unit_under_test = KindPattern(integer=re.compile('i_.*_var')) + issues = unit_under_test.examine(source) + assert [str(issue) for issue in issues] \ + == ["4: Kind '' found for integer variable 'global_int' " + "does not fit the pattern /i_.*_var/.", + "8: Kind '' found for integer variable 'member_int' " + "does not fit the pattern /i_.*_var/.", + "17: Kind '' found for integer variable 'arg_int' " + "does not fit the pattern /i_.*_var/.", + "20: Kind '' found for integer variable 'return_int' " + "does not fit the pattern /i_.*_var/.", + "27: Kind '' found for integer variable 'marg_int' " + "does not fit the pattern /i_.*_var/."] + + def test_missing_kind_float(self, missing_kind_text): + reader = SourceStringReader(missing_kind_text) + source = FortranSource(reader) + unit_under_test = KindPattern(real=re.compile('r_.*_var')) + issues = unit_under_test.examine(source) + assert [str(issue) for issue in issues] \ + == ["5: Kind '' found for real variable 'global_float' " + "does not fit the pattern /r_.*_var/.", + "9: Kind '' found for real variable 'member_float' " + "does not fit the pattern /r_.*_var/.", + "18: Kind '' found for real variable 'arg_float' " + "does not fit the pattern /r_.*_var/.", + "28: Kind '' found for real variable 'marg_float' " + "does not fit the pattern /r_.*_var/.", + "30: Kind '' found for real variable 'return_float' " + "does not fit the pattern /r_.*_var/."] From 023640a92a8cfcd85e67dfd88f9aa187e411b738 Mon Sep 17 00:00:00 2001 From: mo-lucy-gordon <120176477+mo-lucy-gordon@users.noreply.github.com> Date: Mon, 27 Nov 2023 17:21:43 +0000 Subject: [PATCH 06/10] Made values dynamic in pyproject.toml (#135) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 550d836..0c4d5fc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,7 @@ requires-python = '>=3.7, <4' license = {text = 'BSD 3-Clause License'} classifiers = ['Programming Language :: Python :: 3'] dependencies = ['fparser >= 0.1.2'] -dynamic = ['version'] +dynamic = ['version', 'readme'] [project.optional-dependencies] From 03c672da69087f49f8c49ad2dc899eafe42e9d3e Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Mon, 27 Nov 2023 17:32:07 +0000 Subject: [PATCH 07/10] Update performance.yml See if we can enforce ordering on the workflows. --- .github/workflows/performance.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml index 9b68e3f..f7de294 100644 --- a/.github/workflows/performance.yml +++ b/.github/workflows/performance.yml @@ -4,7 +4,9 @@ on: push: branches: - master - + workflow_run: + workflows: ["sphinx"] + types: [completed] jobs: From fffc2a8d64f7bbf11356196a3bb8e209e6a334dc Mon Sep 17 00:00:00 2001 From: Sam Clarke-Green <74185251+t00sa@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:49:55 +0000 Subject: [PATCH 08/10] Improve CLI handling of configuration file arguments (#131) * Make -configuration option compulsory This updates the stylist CLI to make the -configuration compulsory. This will cause the command to fail early if the configuration is incorrect, allowing ArgumentParser to display a meaningful error message. * Check configuration exists and has a valid extension This updates the CLI to check whether the configuration file exists and whether it has a valid python extension. This allows ArgumentParser to display a meaningful error if the configuration is not valid. * Add name to list of contributors As per the guidance, I have added my name to the list of contributors. * Allow Configurations to be combined Create an overload method which shallow merges the pipes and styles attributes of a second Configuration into the current instance. This is intended to be used to allow per-site, per-user, and per-project configurations to be combined into a single entity. * Adapt configuration option and improve testing Reviewer has suggested changing the argument handling to prevent the user from having to specify an option because this is contrary to spirit of making the flag voluntary. Instead, the code checks a variety of locations before raising an exception if the configuration cannot be found. This also improves temporary directory handling in tests as recommended by the reviewer, replacing clunky custom code with a function provide pytest. * Fix type hinting of __configure() The __configure() function return value in the main script had the wrong type hinting. This changes the hinting to match None in the event of a configuration problem. --- CONTRIBUTING.md | 1 + source/stylist/__main__.py | 40 ++++++++++-- source/stylist/configuration.py | 6 ++ unit-tests/__main__test.py | 101 ++++++++++++++++++++++++++++++- unit-tests/configuration_test.py | 54 +++++++++++++++++ 5 files changed, 194 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 873ee17..12d1737 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,6 +28,7 @@ below: * Matt Shin (Met Office, UK) * Bilal Chughtai (Met Office, UK) * James Cuninngham-Smith (Met Office, UK) +* Sam Clarke-Green (Met Office, UK) (All contributors are identifiable with email addresses in the version control logs or otherwise.) diff --git a/source/stylist/__main__.py b/source/stylist/__main__.py index 1e20b29..3ad0716 100755 --- a/source/stylist/__main__.py +++ b/source/stylist/__main__.py @@ -13,7 +13,7 @@ from pathlib import Path import sys from textwrap import indent -from typing import List, Sequence +from typing import List, Sequence, Union from stylist import StylistException from stylist.configuration import (Configuration, @@ -25,6 +25,11 @@ from stylist.style import Style +# Paths to site-wide and per-user style files +site_file = Path("/etc/stylist.py") +user_file = Path.home() / ".stylist.py" + + def __parse_cli() -> argparse.Namespace: """ Parse the command line for stylist arguments. @@ -107,10 +112,30 @@ def __process(candidates: List[Path], styles: Sequence[Style]) -> List[Issue]: return issues -def __configure(project_file: Path) -> Configuration: - configuration = load_configuration(project_file) - # TODO /etc/fab.ini - # TODO ~/.fab.ini - Path.home() / '.fab.ini' +def __configure(project_file: Path) -> Union[Configuration, None]: + """ + Load configuration styles in order of specificity + + Load the global site configuration, the per-user configuration, and + finally the configuration option provided on the command line. + More specific options are allowed to override more general ones, + allowing a configuration to built up gradually. + """ + + candidates = [site_file, user_file, project_file] + + configuration = None + + for target in candidates: + if target is None or not target.exists(): + continue + + style = load_configuration(target) + if configuration is None: + configuration = style + else: + configuration.overload(style) + return configuration @@ -187,6 +212,11 @@ def main() -> None: logger.setLevel(logging.WARNING) configuration = __configure(arguments.configuration) + if configuration is None: + # No valid configuration files have been found + # FIXME: proper exit handling + raise Exception("no valid style files found") + issues = perform(configuration, arguments.source, arguments.style, diff --git a/source/stylist/configuration.py b/source/stylist/configuration.py index ad025b5..84a37bd 100644 --- a/source/stylist/configuration.py +++ b/source/stylist/configuration.py @@ -9,6 +9,8 @@ Configuration may be defined by software or read from a Windows .ini file. """ +from __future__ import annotations + from importlib.util import module_from_spec, spec_from_file_location from pathlib import Path from typing import Dict, List, Tuple, Type @@ -37,6 +39,10 @@ def add_pipe(self, extension: str, pipe: FilePipe): def add_style(self, name: str, style: Style): self._styles[name] = style + def overload(self, other: Configuration) -> None: + self._pipes = {**self._pipes, **other._pipes} + self._styles = {**self._styles, **other._styles} + @property def file_pipes(self) -> Dict[str, FilePipe]: return self._pipes diff --git a/unit-tests/__main__test.py b/unit-tests/__main__test.py index 1f9cff8..f715ecb 100644 --- a/unit-tests/__main__test.py +++ b/unit-tests/__main__test.py @@ -7,12 +7,13 @@ """ Tests the reporting of error conditions. """ -from pathlib import Path -from pytest import raises +from pathlib import Path +from pytest import raises, fixture from stylist import StylistException -from stylist.__main__ import perform +from stylist.__main__ import perform, __configure +import stylist.__main__ as maintest from stylist.configuration import Configuration from stylist.style import Style @@ -40,3 +41,97 @@ def test_missing_style(tmp_path: Path): _ = perform(configuration, [tmp_path], ['missing'], [], verbose=False) assert ex.value.message \ == 'Style "missing" is not defined by the configuration.' + + +@fixture(scope="session") +def site_config(tmp_path_factory): + site = tmp_path_factory.mktemp("data") / "site.py" + site.write_text("\n".join([ + "from stylist.source import FilePipe, PlainText", + "from stylist.rule import LimitLineLength", + "from stylist.style import Style", + "txt = FilePipe(PlainText)", + "foo = Style(LimitLineLength(80))", + ])) + + return site + + +@fixture(scope="session") +def user_config(tmp_path_factory): + user = tmp_path_factory.mktemp("data") / "user.py" + user.write_text("\n".join([ + "from stylist.source import FilePipe, PlainText", + "from stylist.rule import TrailingWhitespace", + "from stylist.style import Style", + "txt = FilePipe(PlainText)", + "bar = Style(TrailingWhitespace())", + ])) + + return user + + +@fixture(scope="session") +def project_config(tmp_path_factory): + project = tmp_path_factory.mktemp("data") / "project.py" + project.write_text("\n".join([ + "from stylist.source import FilePipe, PlainText", + "from stylist.rule import LimitLineLength, TrailingWhitespace", + "from stylist.style import Style", + "txt = FilePipe(PlainText)", + "foo = Style(LimitLineLength(80), TrailingWhitespace)", + ])) + + return project + + +def test_no_configurations(tmp_path): + + maintest.site_file = None + maintest.user_file = None + + configuration = __configure(None) + assert configuration is None + + +def test_site_only_configuration(site_config): + + maintest.site_file = site_config + maintest.user_file = None + + configuration = __configure(None) + assert configuration is not None + assert list(configuration.styles) == ["foo"] + assert len(configuration.styles["foo"].list_rules()) == 1 + + +def test_user_only_configuration(user_config): + + maintest.site_file = None + maintest.user_file = user_config + + configuration = __configure(None) + assert configuration is not None + assert list(configuration.styles) == ["bar"] + assert len(configuration.styles["bar"].list_rules()) == 1 + + +def test_user_and_site_configurations(site_config, user_config): + + maintest.site_file = site_config + maintest.user_file = user_config + + configuration = __configure(None) + assert configuration is not None + assert list(configuration.styles) == ["foo", "bar"] + + +def test_all_configurations(site_config, user_config, project_config): + + maintest.site_file = site_config + maintest.user_file = user_config + + configuration = __configure(project_config) + assert configuration is not None + assert list(configuration.styles) == ["foo", "bar"] + assert len(configuration.styles["foo"].list_rules()) == 2 diff --git a/unit-tests/configuration_test.py b/unit-tests/configuration_test.py index 2755b95..2e48196 100644 --- a/unit-tests/configuration_test.py +++ b/unit-tests/configuration_test.py @@ -177,3 +177,57 @@ def test_regex_rule(self, tmp_path: Path): assert len(style.list_rules()) == 1 assert isinstance(style.list_rules()[0], DummyRuleOne) assert cast(DummyRuleOne, style.list_rules()[0]).first.pattern == r'.*' + + def test_config_add_pipes(self, tmp_path: Path): + first_file = tmp_path / 'first.py' + first_file.write_text(dedent(""" + from stylist.source import FilePipe + from configuration_test import DummySource + foo = FilePipe(DummySource) + """)) + + second_file = tmp_path / 'second.py' + second_file.write_text(dedent(""" + from stylist.source import FilePipe + from configuration_test import DummyProcOne, DummySource + foo = FilePipe(DummySource, DummyProcOne) + """)) + + configuration = load_configuration(first_file) + assert list(configuration.file_pipes.keys()) == ['foo'] + style = configuration.file_pipes['foo'] + assert style.parser == DummySource + assert len(style.preprocessors) == 0 + assert configuration.styles == {} + + configuration.overload(load_configuration(second_file)) + assert list(configuration.file_pipes.keys()) == ['foo'] + style = configuration.file_pipes['foo'] + assert style.parser == DummySource + assert len(style.preprocessors) == 1 + assert configuration.styles == {} + + def test_config_add_styles(self, tmp_path: Path): + first_file = tmp_path / 'first.py' + first_file.write_text(dedent(""" + from stylist.style import Style + from configuration_test import DummyRuleZero + foo = Style(DummyRuleZero()) + """)) + + second_file = tmp_path / 'second.py' + second_file.write_text(dedent(""" + from stylist.style import Style + from configuration_test import DummyRuleOne + foo = Style(DummyRuleOne(1)) + """)) + + configuration = load_configuration(first_file) + assert list(configuration.styles.keys()) == ['foo'] + style = configuration.styles['foo'] + assert isinstance(style.list_rules()[0], DummyRuleZero) + + configuration.overload(load_configuration(second_file)) + assert list(configuration.styles.keys()) == ['foo'] + style = configuration.styles['foo'] + assert isinstance(style.list_rules()[0], DummyRuleOne) From 900b9dfdba164797a4e445cca816c42978905e85 Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Thu, 4 Jan 2024 16:45:21 +0000 Subject: [PATCH 09/10] Restore long description (#139) --- pyproject.toml | 11 +++++++++-- setup.cfg | 18 ------------------ 2 files changed, 9 insertions(+), 20 deletions(-) delete mode 100644 setup.cfg diff --git a/pyproject.toml b/pyproject.toml index 0c4d5fc..7f8664b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,9 +7,16 @@ name='stylist' description="Flexible source code style checking tool" requires-python = '>=3.7, <4' license = {text = 'BSD 3-Clause License'} -classifiers = ['Programming Language :: Python :: 3'] dependencies = ['fparser >= 0.1.2'] dynamic = ['version', 'readme'] +keywords = ['linter', 'fortran'] +classifiers = [ + 'Development Status :: 3 - Alpha', + 'Intended Audience :: Developers', + 'Topic :: Software Development :: Quality Assurance', + 'License :: OSI Approved :: BSD License', + 'Programming Language :: Python :: 3' +] [project.optional-dependencies] @@ -31,7 +38,7 @@ documentation = 'https://metoffice.github.io/stylist' repository = 'https://github.com/MetOffice/stylist/' [tool.setuptools.dynamic] -readme = {file = 'README.md'} +readme = {file = 'README.rst'} version = {attr = 'stylist.__version__'} [tool.setuptools.packages.find] diff --git a/setup.cfg b/setup.cfg deleted file mode 100644 index e92c9a0..0000000 --- a/setup.cfg +++ /dev/null @@ -1,18 +0,0 @@ -[metadata] -name = stylist -description = Extensible code style checker currently supporting Fortran, PSyclone DSL, etc -long_description = file:README.rst -long_description_content_type = text/x-rst -url = https://github.com/MetOffice/stylist -author = Met Office -license = BSD 3-Clause -license_files = LICENSE -keywords = linter fortran psyclone -classifiers=Development Status :: 3 - Alpha - Intended Audience :: Developers - Topic :: Software Development :: Quality Assurance - License :: OSI Approved :: BSD License - Programming Language :: Python :: 3.7 - Programming Language :: Python :: 3.8 - Programming Language :: Python :: 3.9 - Programming Language :: Python :: 3.10 From b121ef846eb0ac8a02eff469b04dbc16d7f034cb Mon Sep 17 00:00:00 2001 From: Matthew Hambley Date: Mon, 20 May 2024 08:22:48 +0100 Subject: [PATCH 10/10] Adopt documentation theme from pyData (#141) * Replace `setup.py` script with `stylist.toml` configuration file. * Turned out the `.toml` file wasn't being used. * Discovered mistake in pull request workflow. * Since `setup.py` has been removed we can't check it with `mypy` * Added Conda Forge badge to ReadMe. * Constrain fParser version. * Remove reference to setup.py from documentation. * Adopy pyData theme for documentation. * Correct theme package name. * Add centre footer. * Downgrade theme to an available one. * Updated Sphinx theme and fixed project for more recent PIP. * Update Python version for Sphinx. --- .github/workflows/sphinx.yml | 2 +- .../source/_templates/crown-copyright.html | 11 +++++++++++ documentation/source/conf.py | 17 +++++++++++++++-- pyproject.toml | 2 +- 4 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 documentation/source/_templates/crown-copyright.html diff --git a/.github/workflows/sphinx.yml b/.github/workflows/sphinx.yml index ce0a47e..9dafb21 100644 --- a/.github/workflows/sphinx.yml +++ b/.github/workflows/sphinx.yml @@ -38,7 +38,7 @@ jobs: - name: Install Python uses: actions/setup-python@v2 with: - python-version: 3.8 + python-version: 3.11 - name: Cache pip uses: actions/cache@v2 diff --git a/documentation/source/_templates/crown-copyright.html b/documentation/source/_templates/crown-copyright.html new file mode 100644 index 0000000..997df31 --- /dev/null +++ b/documentation/source/_templates/crown-copyright.html @@ -0,0 +1,11 @@ +{# Crown copyright is presented differently to normal copyright. #} +{# This template uses details from conf.py #} +{% if show_copyright and copyright %} +

+{% if hasdoc('copyright') %} +{% trans path=pathto('copyright'), copyright=copyright|e %}© Copyright {{copyright}}.{% endtrans %} +{% else %} +{% trans copyright=copyright|e %}© Crown Copyright {{copyright}}.{% endtrans %} +{% endif %} +

+{% endif %} diff --git a/documentation/source/conf.py b/documentation/source/conf.py index 23d6808..133722b 100644 --- a/documentation/source/conf.py +++ b/documentation/source/conf.py @@ -23,7 +23,7 @@ # -- Project information ----------------------------------------------------- project = 'Stylist' -copyright = '2022, Crown Copyright' +copyright = '2024, Met Office. All rights reserved' author = 'Stylist Developers' # Headline version for archiving purposes. @@ -82,7 +82,20 @@ # The theme to use for HTML and HTML Help pages. See the documentation for # a list of builtin themes. # -html_theme = 'sphinx_rtd_theme' +html_theme = 'pydata_sphinx_theme' + +html_theme_options = { + "icon_links": [ + { + "name": "GitHub", + "url": "https://github.com/MetOffice/stylist", + "icon": "fa-brands fa-github" + } + ], + "footer_start": ["crown-copyright"], + "footer_center": ["sphinx-version"], + "footer_end": ["theme-version"] +} # Add any paths that contain custom static files (such as style sheets) here, # relative to this directory. They are copied after the builtin static files, diff --git a/pyproject.toml b/pyproject.toml index 7f8664b..a8ffbc0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ test = ['pytest', 'pytest-cov', 'mypy'] performance = ['pytest', 'pytest-benchmark', 'matplotlib'] docs = ['sphinx < 7.0.0', 'sphinx-autodoc-typehints', - 'sphinx-rtd-theme>=1.2.2'] + 'pydata-sphinx-theme>=0.15.2'] release = ['setuptools', 'wheel', 'twine'] [project.scripts]