Merge branch 'master' into NewLook

MetOffice · May 18, 2024 · 325babe · 325babe
2 parents 635bc76 + 900b9df
commit 325babe
Show file tree

Hide file tree

Showing 15 changed files with 511 additions and 68 deletions.
diff --git a/.github/bin/update-index 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 <index file> <release name>
+##############################################################################
+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('''<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
+   "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">''', file=fhandle)
+        print(document_text, file=fhandle)
diff --git 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:

diff --git 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'

diff --git 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/pyproject.toml b/pyproject.toml
@@ -7,16 +7,23 @@ 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]
 dev = ['check-manifest', 'flake8']
 test = ['pytest', 'pytest-cov', 'mypy']
 performance = ['pytest', 'pytest-benchmark', 'matplotlib']
-docs = ['sphinx',
+docs = ['sphinx < 7.0.0',
         'sphinx-autodoc-typehints',
         'pydata-sphinx-theme>=0.15.2']
 release = ['setuptools', 'wheel', 'twine']
@@ -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
diff --git 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
@@ -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/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
@@ -393,6 +392,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
@@ -532,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
@@ -542,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
@@ -575,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)))
 
@@ -601,7 +625,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 +655,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 +682,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]
                 ),
@@ -686,22 +714,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