From add919304ce3f1dab7c8da01e4ab4d1b19bc9196 Mon Sep 17 00:00:00 2001 From: Wyatt Childers Date: Thu, 5 Nov 2026 21:23:14 +0000 Subject: [PATCH 1/3] Improve pre-push checks and correct historic mistakes [] This change introduces an improved version of the pre-push checker that: - allows the spell checking functions to work better with CamelCase names, - adds a per-directory dictionary to allow domain-specific exceptions, - excludes binary files from checks, - understands that md tables, rst blocks, and rst tables cannot be wrapped, - and does not enforce line length on yml, yaml, or json files. Additionally, as the prior file state is assumed to contain no spelling mistakes, a pass has been taken to correct spelling mistakes so further mistakes of the same kind are caught. --- .agents/skills/edg-changes-example/SKILL.md | 6 +- .../skills/edg-spelling-allowlist/SKILL.md | 126 +++ .git-hooks/pre-push.py | 24 +- .github/workflows/changes-fragments.yml | 4 + .github/workflows/dev-tools-tests.yml | 3 + AGENTS.md | 1 + dev_tools/bin/edg-check-file-changes | 224 +++++- dev_tools/bin/edgy | 2 +- .../pylibs/edgacknowledg/client/__init__.py | 18 +- .../pylibs/edgacknowledg/forge/processor.py | 4 +- .../forge/publisher/acknowledg.py | 2 +- .../edgacknowledg/forge/publisher/pool.py | 6 +- .../edgacknowledg/forge/publisher/server.py | 2 +- .../edgacknowledg/forge/runner/client.py | 4 +- .../edgacknowledg/forge/runner/server.py | 2 +- .../edgacknowledg/forge/sandbox_client.py | 2 +- .../pylibs/edgacknowledg/forge/serving.py | 8 +- .../pylibs/edgacknowledg/forge/workspace.py | 2 +- dev_tools/pylibs/edgshell/__init__.py | 48 +- dev_tools/pylibs/edgspelling/__init__.py | 201 +++++ dev_tools/pylibs/edgspelling/py.typed | 0 dev_tools/pylibs/edgutil/__init__.py | 14 +- .../tests/test_client_cancellation.py | 6 +- .../acknowledg-forge/tests/test_lifecycle.py | 2 +- .../acknowledg-forge/tests/test_pool.py | 2 +- dev_tools/spelling_allowlist.json | 746 ++++++++++++++++++ .../tests/test_edg_check_file_changes.py | 433 ++++++++++ dev_tools/tests/test_edg_collect_changes.py | 6 +- dev_tools/tests/test_edgutil.py | 2 +- dev_tools/tests/tool_tree.py | 38 + dev_tools/typecheck-python.sh | 3 + doc/source/exceptions.rst | 2 +- doc/source/lex_pp.rst | 2 +- include_c++/Changes | 2 +- lib_src/eh.h | 2 +- src/Changes | 60 +- src/decl_spec.c | 2 +- src/decls.c | 2 +- src/expr.c | 2 +- src/exprutil.c | 2 +- src/lang_feat.h | 2 +- src/ms_attrib.c | 4 +- util/Changes | 2 +- 43 files changed, 1908 insertions(+), 117 deletions(-) create mode 100644 .agents/skills/edg-spelling-allowlist/SKILL.md create mode 100644 dev_tools/pylibs/edgspelling/__init__.py create mode 100644 dev_tools/pylibs/edgspelling/py.typed create mode 100644 dev_tools/spelling_allowlist.json create mode 100644 dev_tools/tests/test_edg_check_file_changes.py create mode 100644 dev_tools/tests/tool_tree.py diff --git a/.agents/skills/edg-changes-example/SKILL.md b/.agents/skills/edg-changes-example/SKILL.md index f882316738..e01da5d968 100644 --- a/.agents/skills/edg-changes-example/SKILL.md +++ b/.agents/skills/edg-changes-example/SKILL.md @@ -141,7 +141,11 @@ Steps: in. `edg-exec` runs it in the dev-env container, which has `aspell`.) For misspellings that are really identifiers or terms, add them to an `//allowed_words:` line at the very top of the entry, as the failure - message shows; fix real misspellings instead. + message shows; fix real misspellings instead. `edgspelling.preprocess` + leaves an identifier like `getXValue` whole because it contains a + one-letter part. If aspell reports such an identifier whole, add the + whole identifier to `//allowed_words:`; adding only its parts (`get`, + `X`, `Value`) will not suppress the warning. 5. Optionally preview the result without changing anything: `edg-collect-changes --dry-run src/Changes src/pending_changes`. diff --git a/.agents/skills/edg-spelling-allowlist/SKILL.md b/.agents/skills/edg-spelling-allowlist/SKILL.md new file mode 100644 index 0000000000..a3af3dea04 --- /dev/null +++ b/.agents/skills/edg-spelling-allowlist/SKILL.md @@ -0,0 +1,126 @@ +--- +name: edg-spelling-allowlist +description: >- + How the pre-push spell check (edg-check-file-changes, aspell, edgspelling) + decides what to report, and how to add words to the hierarchical spelling + allowlist (dev_tools/spelling_allowlist.json). Use when the pre-push hook + or edg-collect-changes --check reports misspelled words, when choosing + where a new allowed word belongs, or when editing the allowlist file. +--- + +# Spell check and the spelling allowlist + +## How the check works + +The pre-push hook (`.git-hooks/pre-push.py`) runs +`dev_tools/bin/edg-check-file-changes` on every changed file, through +`edg-exec` when docker is preferred. Its spelling pass: + +- Runs `aspell list` on the text. For code files (`.c`, `.cpp`, `.h`, + `.hpp`) only the comments are checked (extracted with `get-comments`). + Other files are checked whole. +- First splits identifiers into words (`edgspelling.preprocess`): `fooBar` + becomes `foo Bar`. Underscores already split words, so `foo_bar` is + checked as `foo` and `bar`. +- Reports only words that are **new** relative to the old version of the + file, minus the allowlist words for that file's path. +- For pending Changes entries (`*/pending_changes/*.txt`), also accepts + the entry's `//allowed_words:` directives and any word already in the + target `Changes` file (see skill **edg-changes-example**). CI runs the + same check with `edg-collect-changes --check`. + +The hook skips some paths entirely (`EXCLUDES_PATH_PREFIXES` in +`.git-hooks/pre-push.py`), including `AGENTS.md`, `.agents/`, and the +allowlist itself. + +## The allowlist file + +`dev_tools/spelling_allowlist.json` is a tree of nodes. Each node may have: + +| Key | Meaning | +| --- | --- | +| `words` | Sorted lowercase words, allowed in any capitalization | +| `names` | Sorted words allowed only with exactly this capitalization | +| `paths` | Object mapping one path component (directory or file name) to a child node | + +A file's allowed words are the union of the nodes on the way down to it, so a +word in the root node is allowed everywhere. The format is documented in +`dev_tools/pylibs/edgspelling/__init__.py`. + +Rules that `edgspelling.Allowlist` enforces when the file loads (a bad +file makes the check refuse to run and fails `ShippedAllowlistTests` in CI): + +- Each list is sorted by code point and has no duplicates. +- `words` entries are lowercase. Use `names` when the capitalization must + match (proper nouns like `EDG` or `Coverity`). +- Only `words`, `names`, and `paths` keys are allowed. +- Each entry is one word. `foo_bar` or `fooBar` would never match, because + identifiers are split before lookup. Add the parts instead. + +## Choosing where a word goes + +Put a word in the **narrowest node** that covers the places it's used +often (see "Prompting is the default" below): + +- **Root:** terms used across the project (C++ terminology, `src`, `utf`). +- **A directory node** (`src`, `dev_tools`, `dev_tools/pylibs/edgtest`, + `doc/source`, …): jargon used only there. Common Python names like + `isinstance` and `pathlib`, and the pylib package names (`edgutil`, …), + live in `dev_tools`. The rare uses elsewhere (`dev-init.py`, + `.git-hooks/`, `doc/source/conf.py`) are left to prompt. + +A word used in only one file almost never earns an entry. The check +reports only words that are new to a file, so the entry would save a prompt +only when the word shows up in a second file. A word used only in a +`Changes` file never needs one, because pending entries already accept +every word in their target `Changes` file. +- **A file node:** a one-off term in a single file. + +### Prompting is the default + +Each allowed word is a blind spot: a typo that happens to match it is never +reported anywhere the entry applies. A broader node makes the blind spot +bigger. Being prompted again costs little, because the check reports only +words that are new to a file. Weigh the two: + +- Prefer leaving a word out and letting people be prompted. Add it only when + people would otherwise be prompted for it very often (for example, a term + used constantly across a directory or the project). +- When a word does earn an entry, choose the narrowest node that stops the + frequent prompts, even if rare uses elsewhere still prompt. For example, + the Python names are in `dev_tools` rather than at the root. +- Be most careful with short words and words that are one letter away from + a common English word (`len`, `str`, `dir`). They're the likeliest to hide + a real typo. + +Don't add a real misspelling. Fix the text. For a Changes entry, prefer an +`//allowed_words:` directive over a new allowlist word unless the term will +recur. + +## Editing the file + +Keep the existing formatting (one-space JSON indent, UTF-8, non-ASCII kept as +is). A Python round trip keeps it exact and keeps the lists sorted: + +```python +import json, pathlib +path = pathlib.Path('dev_tools/spelling_allowlist.json') +tree = json.loads(path.read_text(encoding = 'utf-8')) +tree['words'] = sorted(set(tree['words']) | {'newword'}) +# For a nested node: tree['paths']['src']['words'] = ... +path.write_text( + json.dumps(tree, indent = 1, ensure_ascii = False) + '\n', + encoding = 'utf-8' +) +``` + +Then check that the file still loads, and run the tests: + +```sh +PYTHONPATH=dev_tools/pylibs python3 -c 'import edgspelling; edgspelling.Allowlist()' +PYTHONPATH=dev_tools/pylibs python3 -m unittest discover -s dev_tools/tests \ + -p 'test_edg_check_file_changes.py' +``` + +To see what aspell rejects in some text: +`printf '%s\n' word1 word2 | aspell list`. diff --git a/.git-hooks/pre-push.py b/.git-hooks/pre-push.py index 31b1152d41..8a4dd4c571 100755 --- a/.git-hooks/pre-push.py +++ b/.git-hooks/pre-push.py @@ -50,6 +50,7 @@ ) import edgchanges +import edgshell import edgutil if platform.system() == 'Windows': @@ -94,7 +95,9 @@ 'src/error_msg.txt', # Don't check AGENT files. 'AGENTS.md', - '.agents/' + '.agents/', + # Don't check the spelling allowlist; it is made of words aspell rejects. + 'dev_tools/spelling_allowlist.json' ] def _is_excluded_file_path(path_str: str) -> bool: @@ -230,9 +233,20 @@ def check_changed_files(self, first_new_commit): remote_head = f"{first_new_commit}~1" print('Checking changed files:') - changed_files = git_output( - ['diff', '--no-commit-id', '--name-only', '-r', f"{remote_head}..HEAD"] - ).splitlines() + # Use -z so that git doesn't C-quote paths with unusual characters (e.g., + # non-ASCII ones): the quoted form isn't a usable path. + changed_files = [ + path for path in run_git( + ['diff', '--no-commit-id', '--name-only', '-r', '-z', + f"{remote_head}..HEAD"] + ).stdout.split('\0') if path + ] + + # Binary files have no text to check, so don't extract them. Ask git + # rather than sniffing the contents so that .gitattributes is honored. + binary_files = edgshell.list_binary_files_in_git_diff( + f"{remote_head}..HEAD" + ) mono_repo_dir = edgutil.find_mono_repo_or_exit() containing_tmp_dir = mono_repo_dir / '.tmp' @@ -242,6 +256,8 @@ def check_changed_files(self, first_new_commit): for file_path_str in changed_files: if _is_excluded_file_path(file_path_str): continue + if file_path_str in binary_files: + continue file_path = Path(file_path_str) named_tmp_args = { diff --git a/.github/workflows/changes-fragments.yml b/.github/workflows/changes-fragments.yml index 1d66f33989..6a5acdc2e6 100644 --- a/.github/workflows/changes-fragments.yml +++ b/.github/workflows/changes-fragments.yml @@ -22,7 +22,9 @@ on: - dev_tools/bin/edg-check-file-changes - dev_tools/bin/edg-collect-changes - dev_tools/pylibs/edgchanges/** + - dev_tools/pylibs/edgspelling/** - dev_tools/pylibs/edgutil/** + - dev_tools/spelling_allowlist.json pull_request: paths: - '**/pending_changes/**' @@ -30,7 +32,9 @@ on: - dev_tools/bin/edg-check-file-changes - dev_tools/bin/edg-collect-changes - dev_tools/pylibs/edgchanges/** + - dev_tools/pylibs/edgspelling/** - dev_tools/pylibs/edgutil/** + - dev_tools/spelling_allowlist.json permissions: contents: read diff --git a/.github/workflows/dev-tools-tests.yml b/.github/workflows/dev-tools-tests.yml index 060354ed29..f83db9f7c9 100644 --- a/.github/workflows/dev-tools-tests.yml +++ b/.github/workflows/dev-tools-tests.yml @@ -16,7 +16,10 @@ on: - dev_tools/bin/edg-check-file-changes - dev_tools/bin/edg-collect-changes - dev_tools/pylibs/edgchanges/** + - dev_tools/pylibs/edgshell/** + - dev_tools/pylibs/edgspelling/** - dev_tools/pylibs/edgutil/** + - dev_tools/spelling_allowlist.json - dev_tools/tests/** merge_group: diff --git a/AGENTS.md b/AGENTS.md index c21ffc886b..74c548f3a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -149,5 +149,6 @@ If an agent does not auto-load skills, open the matching | `edg-cpfe-style` | [`.agents/skills/edg-cpfe-style/SKILL.md`](.agents/skills/edg-cpfe-style/SKILL.md) | Front-end coding style (`doc/source/code_style.rst`) | | `edg-python-style` | [`.agents/skills/edg-python-style/SKILL.md`](.agents/skills/edg-python-style/SKILL.md) | Python under `dev_tools/` (no formal RST; match in-tree) | | `edg-changes-example` | [`.agents/skills/edg-changes-example/SKILL.md`](.agents/skills/edg-changes-example/SKILL.md) | Adding a Changes entry (`pending_changes/`); encoding non-UTF-8 examples in any `Changes` file | +| `edg-spelling-allowlist` | [`.agents/skills/edg-spelling-allowlist/SKILL.md`](.agents/skills/edg-spelling-allowlist/SKILL.md) | Spell check reports from pre-push or `edg-collect-changes --check`; adding words to `dev_tools/spelling_allowlist.json` | | `acknowledg-client` | [`.agents/skills/acknowledg-client/SKILL.md`](.agents/skills/acknowledg-client/SKILL.md) | Batch/Direct client, reconnect, provider CLIs | | `acknowledg-bench` | [`.agents/skills/acknowledg-bench/SKILL.md`](.agents/skills/acknowledg-bench/SKILL.md) | `.cgout` / Ir / baselines / bench review | diff --git a/dev_tools/bin/edg-check-file-changes b/dev_tools/bin/edg-check-file-changes index bd022df514..9c0b868b97 100755 --- a/dev_tools/bin/edg-check-file-changes +++ b/dev_tools/bin/edg-check-file-changes @@ -6,7 +6,6 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import argparse -import contextlib import functools import re import json @@ -21,6 +20,7 @@ from pathlib import Path from typing import Any, List, Optional, Set, Tuple import edgchanges +import edgspelling import edgutil TOOL_PATH_ASPELL = shutil.which('aspell') @@ -28,6 +28,10 @@ TOOL_PATH_GET_COMMENTS = edgutil.RequiredToolPath('get-comments') CODE_FILE_EXTENSIONS = {'.c', '.cpp', '.h', '.hpp'} +# Text and data files where long lines are normal; these skip the line length +# check. +NO_LINE_LENGTH_EXTENSIONS = {'.txt', '.json', '.yml', '.yaml'} + # (pattern, ignore_case, exclude_substrings) _FIND_THE_THE_CHECKS = [ ('the the', True, ()), @@ -110,31 +114,54 @@ def _is_aspell_working() -> bool: return completed_process.returncode == 0 def _run_spell_command(file_path: Path) -> Set[str]: - with contextlib.ExitStack() as exit_stack: - src_file_handle = exit_stack.enter_context(open(file_path, 'rb')) + '''Return the words aspell rejects in file_path. + + Only the comments of code files are checked. The text is preprocessed + first (see edgspelling.preprocess). + ''' + with open(file_path, 'rb') as src_file_handle: if file_path.suffix in CODE_FILE_EXTENSIONS: - comment_rewrite_file = exit_stack.enter_context(tempfile.TemporaryFile()) completed_process = subprocess.run( [str(TOOL_PATH_GET_COMMENTS), '-c'], stdin = src_file_handle, - stdout = comment_rewrite_file, + stdout = subprocess.PIPE, stderr = subprocess.STDOUT ) - spelling_input = comment_rewrite_file + spelling_bytes = completed_process.stdout else: - spelling_input = src_file_handle + spelling_bytes = src_file_handle.read() - completed_process = subprocess.run( - [TOOL_PATH_ASPELL, 'list'], - stdin = spelling_input, - stdout = subprocess.PIPE, - stderr = subprocess.STDOUT - ) + spelling_text = edgspelling.preprocess( + spelling_bytes.decode('utf-8', errors = 'replace') + ) + completed_process = subprocess.run( + [TOOL_PATH_ASPELL, 'list'], + input = spelling_text.encode('utf-8'), + stdout = subprocess.PIPE, + stderr = subprocess.STDOUT + ) - return set(completed_process.stdout.decode().splitlines()) + return set(completed_process.stdout.decode().splitlines()) -def check_spelling(old_file: Path, new_file: Path) -> Optional[List[str]]: - '''Report new misspellings and "the the" typos relative to the old file.''' +def _without_allowed_words(allowlist: edgspelling.Allowlist, + words: Set[str], + repo_path: str) -> Set[str]: + '''Drop the words allowlist allows for repo_path.''' + allowed_words = allowlist.words_for_path(repo_path) + allowed_names = allowlist.names_for_path(repo_path) + return { + word for word in words + if word.lower() not in allowed_words and word not in allowed_names + } + +def check_spelling(allowlist: edgspelling.Allowlist, + repo_path: str, + old_file: Path, + new_file: Path) -> Optional[List[str]]: + '''Report new misspellings relative to the old file. + + Words allowlist allows for repo_path aren't reported. + ''' if not _is_aspell_working(): return None @@ -142,7 +169,10 @@ def check_spelling(old_file: Path, new_file: Path) -> Optional[List[str]]: new_mispelled_words = _run_spell_command(new_file) old_mispelled_words = _run_spell_command(old_file) - return sorted(new_mispelled_words - old_mispelled_words) + newly_mispelled_words = new_mispelled_words - old_mispelled_words + return sorted( + _without_allowed_words(allowlist, newly_mispelled_words, repo_path) + ) @functools.lru_cache(maxsize = None) def _changes_file_misspellings(changes_path: Path) -> Set[str]: @@ -152,12 +182,14 @@ def _changes_file_misspellings(changes_path: Path) -> Set[str]: return set() return _run_spell_command(changes_path) -def _check_fragment_spelling(repo_path: str, +def _check_fragment_spelling(allowlist: edgspelling.Allowlist, + repo_path: str, new_file: Path) -> Optional[List[str]]: '''Report misspellings in a Changes entry. The directives and the tag block aren't checked, and words the entry's //allowed_words: directives list aren't reported. Neither are words + the allowlist allows for the entry's path, nor words already in the Changes file the entry will be collected into (so a term like "constexpr" needn't be allowed in every entry). ''' @@ -176,7 +208,11 @@ def _check_fragment_spelling(repo_path: str, target_path = edgchanges.target_changes_path(Path.cwd() / repo_path) accepted_words = allowed_words | _changes_file_misspellings(target_path) - return sorted(mispelled_words - accepted_words) + return sorted( + _without_allowed_words( + allowlist, mispelled_words - accepted_words, repo_path + ) + ) def _file_check_config(file_path: Path): '''Return spelling/coding/line-length check settings for a path.''' @@ -188,7 +224,7 @@ def _file_check_config(file_path: Path): if suffix in CODE_FILE_EXTENSIONS: check_for_coding_errors = True check_for_overlong_lines = True - elif suffix == '.txt': + elif suffix in NO_LINE_LENGTH_EXTENSIONS: pass else: check_for_overlong_lines = True @@ -223,14 +259,132 @@ def _check_coding_errors(new_file: Path) -> Any: return coding_errors +MD_TABLE_DELIMITER_CHARS = set('|:- \t') +RST_GRID_TABLE_BORDER_REGEX = re.compile(r'^\s*\+([-=]+\+)+\s*$') +RST_SIMPLE_TABLE_BORDER_REGEX = re.compile(r'^\s*=+([ \t]+=+)+\s*$') + +def _is_md_table_delimiter_row(stripped_line: str) -> bool: + '''Return True if stripped_line looks like the delimiter row of a pipe + table (e.g., "| --- | :-: |").''' + return ( + '-' in stripped_line + and set(stripped_line) <= MD_TABLE_DELIMITER_CHARS + ) + +def _md_table_line_numbers(lines: List[str]) -> Set[int]: + '''Return the (1-based) numbers of the lines of the pipe tables in lines. + + This is an approximation: a table is a run of lines starting with "|" (after + any indentation) that includes a delimiter row. + ''' + table_line_numbers = set() + index = 0 + while index < len(lines): + if not lines[index].lstrip().startswith('|'): + index += 1 + continue + run_start = index + while index < len(lines) and lines[index].lstrip().startswith('|'): + index += 1 + if any(_is_md_table_delimiter_row(line.strip()) + for line in lines[run_start:index]): + table_line_numbers.update(range(run_start + 1, index + 1)) + return table_line_numbers + +def _is_rst_line_block_line(line: str) -> bool: + '''Return True if line starts a line of an RST line block ("| text", or a + lone "|" for a blank line, after any indentation).''' + stripped_line = line.strip() + return stripped_line == '|' or line.lstrip().startswith(('| ', '|\t')) + +def _indentation(line: str) -> int: + '''Return the width of the leading whitespace of line.''' + return len(line.expandtabs()) - len(line.expandtabs().lstrip()) + +def _rst_unwrappable_line_numbers(lines: List[str]) -> Set[int]: + '''Return the (1-based) numbers of the lines of the grid tables, simple + tables, and line blocks in lines. + + This is an approximation: a grid table starts with a "+---+" border and + continues while lines start with "+" or "|" (after any indentation). A + simple table starts with a border of two or more "=" columns and ends at a + later border that's followed by a blank line or the end of the file. A + line block is a run of lines starting with "| " (or a lone "|"), along with + their continuation lines, which are indented past the "|". Directive + tables (list-table, csv-table) aren't recognized; their cells can be + wrapped. + ''' + def is_blank_or_end(index: int) -> bool: + return index >= len(lines) or not lines[index].strip() + + unwrappable_line_numbers = set() + index = 0 + while index < len(lines): + line = lines[index] + if _is_rst_line_block_line(line): + bar_indentation = _indentation(line) + unwrappable_line_numbers.add(index + 1) + index += 1 + while (index < len(lines) + and lines[index].strip() + and not _is_rst_line_block_line(lines[index]) + and _indentation(lines[index]) > bar_indentation): + unwrappable_line_numbers.add(index + 1) + index += 1 + continue + if RST_GRID_TABLE_BORDER_REGEX.match(line): + table_start = index + index += 1 + while (index < len(lines) + and lines[index].lstrip().startswith(('+', '|'))): + index += 1 + unwrappable_line_numbers.update(range(table_start + 1, index + 1)) + continue + if (RST_SIMPLE_TABLE_BORDER_REGEX.match(line) + and not is_blank_or_end(index + 1)): + table_end = next( + ( + end for end in range(index + 1, len(lines)) + if RST_SIMPLE_TABLE_BORDER_REGEX.match(lines[end]) + and is_blank_or_end(end + 1) + ), + None + ) + if table_end is not None: + unwrappable_line_numbers.update(range(index + 1, table_end + 2)) + index = table_end + 1 + continue + index += 1 + return unwrappable_line_numbers + +def _unwrappable_line_numbers(suffix: str, lines: List[str]) -> Set[int]: + '''Return the (1-based) numbers of the lines that can't be wrapped in lines, + the contents of a file with the given suffix. Only Markdown tables and + reStructuredText tables and line blocks are recognized.''' + if suffix == '.md': + return _md_table_line_numbers(lines) + if suffix == '.rst': + return _rst_unwrappable_line_numbers(lines) + return set() + def _check_overlong_lines(new_file: Path, ignore_trailing_whitespace: bool = False) -> Any: - '''Report lines longer than 79 columns after tab expansion.''' + '''Report lines longer than 79 columns after tab expansion. + + The rows of Markdown and reStructuredText tables and the lines of + reStructuredText line blocks can't be wrapped, so they aren't reported, + though they still appear in the context of other lines. + ''' overlong_lines = {} with open(new_file, 'r', errors = 'replace') as file_handle: lines = file_handle.readlines() + unwrappable_line_numbers = _unwrappable_line_numbers( + new_file.suffix, lines + ) for line_no, line in enumerate(lines, start = 1): + if line_no in unwrappable_line_numbers: + continue checked_line = ( line.rstrip() if ignore_trailing_whitespace else line.rstrip('\n') ) @@ -244,7 +398,9 @@ def _check_overlong_lines(new_file: Path, return overlong_lines -def check_changes_fragment(repo_path: str, new_file: Path) -> Any: +def check_changes_fragment(allowlist: edgspelling.Allowlist, + repo_path: str, + new_file: Path) -> Any: '''Check new_file, the contents of the pending Changes entry repo_path. Besides the usual checks, its format is checked (see edgchanges). Its @@ -260,7 +416,9 @@ def check_changes_fragment(repo_path: str, new_file: Path) -> Any: _, prose_lines = edgchanges.read_prose(new_file) return { 'format_errors': format_errors, - 'misspellings': _check_fragment_spelling(repo_path, new_file), + 'misspellings': _check_fragment_spelling( + allowlist, repo_path, new_file + ), 'typos': _find_the_the_in_lines(prose_lines), 'coding_errors': {}, 'overlong_lines': _check_overlong_lines( @@ -269,17 +427,22 @@ def check_changes_fragment(repo_path: str, new_file: Path) -> Any: ) } -def check_file(repo_path: str, old_file: Path, new_file: Path) -> Any: +def check_file(allowlist: edgspelling.Allowlist, + repo_path: str, + old_file: Path, + new_file: Path) -> Any: '''Check new_file, the new contents of repo_path, against old_file.''' if edgchanges.is_fragment_path(repo_path): - return check_changes_fragment(repo_path, new_file) + return check_changes_fragment(allowlist, repo_path, new_file) check_for_coding_errors, check_for_overlong_lines = ( _file_check_config(new_file) ) return { 'format_errors': [], - 'misspellings': check_spelling(old_file, new_file), + 'misspellings': check_spelling( + allowlist, repo_path, old_file, new_file + ), 'typos': find_the_the(new_file), 'coding_errors': ( _check_coding_errors(new_file) if check_for_coding_errors else {} @@ -310,6 +473,14 @@ def parse_args() -> argparse.Namespace: def main() -> None: args = parse_args() + # Refuse to start with a bad allowlist rather than checking against a + # partial one. + try: + allowlist = edgspelling.Allowlist() + except edgspelling.AllowlistError as error: + edgutil.eprint(f"Invalid spelling allowlist: {error}") + sys.exit(1) + with open(args.input_spec, 'r', encoding = 'utf-8') as file_handle: input_dict = json.load(file_handle) @@ -328,6 +499,7 @@ def main() -> None: for key, (old_file, new_file) in input_dict.items(): output_dict[key] = executor.submit( check_file, + allowlist, key, cwd.joinpath(old_file), cwd.joinpath(new_file) diff --git a/dev_tools/bin/edgy b/dev_tools/bin/edgy index ab216db5e7..260136ccb0 100755 --- a/dev_tools/bin/edgy +++ b/dev_tools/bin/edgy @@ -1661,7 +1661,7 @@ class DynamicLoadingTestRunner: # Yield back to the event loop to work on other work if possible. await asyncio.sleep(0) except asyncio.CancelledError: - # This task cannot be cancelled; it must complete so that the + # This task cannot be canceled; it must complete so that the # expectations can be written correctly. pass diff --git a/dev_tools/pylibs/edgacknowledg/client/__init__.py b/dev_tools/pylibs/edgacknowledg/client/__init__.py index 359255638b..3b742214b9 100644 --- a/dev_tools/pylibs/edgacknowledg/client/__init__.py +++ b/dev_tools/pylibs/edgacknowledg/client/__init__.py @@ -488,7 +488,7 @@ async def add_section(self, handler: ReviewClientHandler) -> None: except NotConnectedError: pass except BaseException: - # Failed or cancelled: not added. + # Failed or canceled: not added. self._review.discard_handler(handler) raise @@ -576,8 +576,8 @@ async def subscribe_section(self, handler: ReviewClientHandler) -> None: channel = self._client._make_channel(subscription_id) await announce_section(channel, handler, self._review) except BaseException: - # Failed or cancelled: claimed by no one. Shielded, so being - # cancelled again can't keep the release from being sent. + # Failed or canceled: claimed by no one. Shielded, so being + # canceled again can't keep the release from being sent. await asyncio.shield(self._client._release(subscription_id)) raise loop = get_running_event_loop() @@ -670,7 +670,7 @@ async def add_section(self, handler: ReviewClientHandler) -> None: try: await self.subscribe_section(handler) except BaseException: - # Failed or cancelled: not added. + # Failed or canceled: not added. self._review.discard_handler(handler) raise @@ -775,7 +775,7 @@ class BatchClient: ``async with BatchClient(...) as session`` starts the connection and yields a ``BatchClientSession``. The ``with`` block waits until ``session.stop()`` - unless cancelled or an exception is raised. + unless canceled or an exception is raised. A server speaking another protocol revision stops the session; with ``wait_for_server_update``, one speaking an older one doesn't: the @@ -1109,13 +1109,13 @@ async def _subscribe(self, payload: Dict[str, Any]) -> uuid.UUID: # (which its keepalive pings notice) fails it. echo = await future except BaseException: - # Failed or cancelled: no longer awaited. + # Failed or canceled: no longer awaited. if self._pending_subscribe.get(key) is future: del self._pending_subscribe[key] if (future.done() and not future.cancelled() and future.exception() is None): # Answered all the same: claimed by no one. Shielded, so being - # cancelled again can't keep the release from being sent. + # canceled again can't keep the release from being sent. await asyncio.shield(self._release( uuid.UUID(str(future.result()['subscription_id'])) )) @@ -1137,7 +1137,7 @@ async def _unsubscribe(self, subscription_id: uuid.UUID) -> None: # Untimed, as subscribing is. await future except BaseException: - # Failed or cancelled: no longer awaited. + # Failed or canceled: no longer awaited. self._pending_unsubscribe.pop(subscription_id, None) if not future.done(): future.cancel() @@ -1191,7 +1191,7 @@ class DirectClient: '''Async context manager for dedicated ``/source`` and ``/edit`` sessions. ``async with DirectClient(...) as session`` yields a ``DirectClientSession``. - The ``with`` block waits until ``session.stop()`` unless cancelled or an + The ``with`` block waits until ``session.stop()`` unless canceled or an exception is raised. ''' diff --git a/dev_tools/pylibs/edgacknowledg/forge/processor.py b/dev_tools/pylibs/edgacknowledg/forge/processor.py index ac17bc9405..53491af47a 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/processor.py +++ b/dev_tools/pylibs/edgacknowledg/forge/processor.py @@ -199,7 +199,7 @@ def busy_pull_requests(self) -> frozenset[int]: async def run(self) -> None: '''Clean up after a restart, then process changes forever (until - cancelled, which cancels what is being processed). + canceled, which cancels what is being processed). ''' await self.restore() last_maintenance = self._clock() @@ -218,7 +218,7 @@ async def run(self) -> None: except asyncio.TimeoutError: pass finally: - # Stopping: what is being processed is cancelled (and its attempt + # Stopping: what is being processed is canceled (and its attempt # refunded) now, while the store, the forge, and the builder are # still open. await self._cancel_in_flight() diff --git a/dev_tools/pylibs/edgacknowledg/forge/publisher/acknowledg.py b/dev_tools/pylibs/edgacknowledg/forge/publisher/acknowledg.py index 4849172e9a..b5754209db 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/publisher/acknowledg.py +++ b/dev_tools/pylibs/edgacknowledg/forge/publisher/acknowledg.py @@ -218,7 +218,7 @@ async def publish(wanted: Wanted, api_key: Optional[str], pool: ProcessPool, reviews_dir: Optional[Path] = None) -> None: - '''Publish what ``wanted`` asks for, until cancelled; raise should the + '''Publish what ``wanted`` asks for, until canceled; raise should the session stop (``IncompatibleProtocolVersion``: AcknowlEDG is newer). ''' async with BatchClient( diff --git a/dev_tools/pylibs/edgacknowledg/forge/publisher/pool.py b/dev_tools/pylibs/edgacknowledg/forge/publisher/pool.py index d0121d16da..c08a4be2bd 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/publisher/pool.py +++ b/dev_tools/pylibs/edgacknowledg/forge/publisher/pool.py @@ -24,7 +24,7 @@ (with whatever it started) and replaced; so is one that exits. Either way the request fails, and the client library answers the reviewer with its failure reply. -- A request abandoned (cancelled, say, as its section is removed) before +- A request abandoned (canceled, say, as its section is removed) before it is given to a process is skipped; one abandoned after is still answered (its process stays busy with it until then), and the answer dropped. @@ -224,7 +224,7 @@ async def answer(self, section: SectionRecord, request: Any) -> Answer: '''Have the next free process answer ``request`` for ``section``; - raise ``PoolError`` if it can't. Cancelling this abandons the request. + raise ``PoolError`` if it can't. Canceling this abandons the request. ''' if all(slot.starting_later() for slot in self._slots): raise PoolError('every pool process is restarting') @@ -248,7 +248,7 @@ async def answer(self, async def _dispatch(self, slot: _Slot) -> None: '''Give ``slot``'s process the queued requests, one at a time, each - once it has answered the last, until cancelled. + once it has answered the last, until canceled. ''' while True: # A running process first (replaced at once if it went), so it takes diff --git a/dev_tools/pylibs/edgacknowledg/forge/publisher/server.py b/dev_tools/pylibs/edgacknowledg/forge/publisher/server.py index 93161c8354..97b89f40fb 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/publisher/server.py +++ b/dev_tools/pylibs/edgacknowledg/forge/publisher/server.py @@ -108,7 +108,7 @@ async def _keep_publishing(settings: 'asyncio.Queue[_Settings]', wanted: Wanted, pool: ProcessPool) -> str: '''Publish what ``wanted`` asks for with the latest ``settings`` a client - sent, until cancelled: from the first ones on; again with new ones, or + sent, until canceled: from the first ones on; again with new ones, or should publishing fail (what it published goes with it; the clients serve it all again on connecting). Except should AcknowlEDG be newer: then it returns why, publishing nothing more until this publisher is diff --git a/dev_tools/pylibs/edgacknowledg/forge/runner/client.py b/dev_tools/pylibs/edgacknowledg/forge/runner/client.py index 8df5eb4160..37983fcf1f 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/runner/client.py +++ b/dev_tools/pylibs/edgacknowledg/forge/runner/client.py @@ -150,7 +150,7 @@ async def run(self, '''Run ``argv`` in the runner (stdout and stderr together), which ends it after ``timeout`` seconds. Its output beyond ``MAX_OUTPUT_BYTES`` is dropped from the middle. - Cancelling this kills the command. + Canceling this kills the command. ''' output = _Output() value = await self._request( @@ -229,7 +229,7 @@ async def _request(self, await connection.send(json.dumps({'id': request_id, **request})) value = await pending.answer except asyncio.CancelledError: - # The caller gave up: kill the command. Shielded, so being cancelled + # The caller gave up: kill the command. Shielded, so being canceled # again can't keep the kill from being sent. await asyncio.shield(self._cancel(connection, request_id)) raise diff --git a/dev_tools/pylibs/edgacknowledg/forge/runner/server.py b/dev_tools/pylibs/edgacknowledg/forge/runner/server.py index c30a4b7faa..ff2b69f115 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/runner/server.py +++ b/dev_tools/pylibs/edgacknowledg/forge/runner/server.py @@ -245,7 +245,7 @@ def create_runner(policy: RunnerPolicy, async def run(connection: Websocket, request_id: int, job: Job) -> None: '''Run ``job``, relaying its output, until it ends, times out, or is - cancelled (by the client, or with its connection). + canceled (by the client, or with its connection). ''' nonlocal active _log.info('running %s', shlex.join(job.argv)) diff --git a/dev_tools/pylibs/edgacknowledg/forge/sandbox_client.py b/dev_tools/pylibs/edgacknowledg/forge/sandbox_client.py index 5dc73ae25a..6849896684 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/sandbox_client.py +++ b/dev_tools/pylibs/edgacknowledg/forge/sandbox_client.py @@ -148,7 +148,7 @@ def _retry(self, ex: Exception) -> Optional[Exception]: return None async def _keep_connected(self) -> None: - '''Keep a connection to the server, until cancelled.''' + '''Keep a connection to the server, until canceled.''' async for connection in connect( self.url, open_timeout = self._open_timeout, diff --git a/dev_tools/pylibs/edgacknowledg/forge/serving.py b/dev_tools/pylibs/edgacknowledg/forge/serving.py index 03abce8fb8..f5e54779e2 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/serving.py +++ b/dev_tools/pylibs/edgacknowledg/forge/serving.py @@ -92,14 +92,14 @@ def url(self, scheme: str = 'ws') -> str: async def wait(self) -> None: '''Until serving stops (with ``close``, or failing).''' - # Shielded: a waiter giving up (being cancelled) mustn't cut the serving - # off; ``close`` stops it gracefully (as ``serve`` does once cancelled). + # Shielded: a waiter giving up (being canceled) mustn't cut the serving + # off; ``close`` stops it gracefully (as ``serve`` does once canceled). await asyncio.shield(self._task) async def close(self) -> None: self._stop.set() try: - # Shielded: if this is cancelled, the shutdown just started still + # Shielded: if this is canceled, the shutdown just started still # finishes; only running out of time cuts it short (below). await asyncio.wait_for(asyncio.shield(self._task), _CLOSE_TIMEOUT) except asyncio.TimeoutError: @@ -161,7 +161,7 @@ async def serve(*blueprints: Blueprint, host: str, port: int, max_message_size: Optional[int] = None) -> None: - '''Serve the app of ``blueprints`` (as ``start``) until cancelled.''' + '''Serve the app of ``blueprints`` (as ``start``) until canceled.''' listening = await start(*blueprints, host = host, port = port, max_message_size = max_message_size) try: diff --git a/dev_tools/pylibs/edgacknowledg/forge/workspace.py b/dev_tools/pylibs/edgacknowledg/forge/workspace.py index 6079f52b22..81b0634f77 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/workspace.py +++ b/dev_tools/pylibs/edgacknowledg/forge/workspace.py @@ -705,7 +705,7 @@ async def _communicate_bounded(process: asyncio.subprocess.Process, limit: int) -> Optional[tuple[bytes, bytes]]: '''``process``'s output (stdout, stderr), once it exits; ``None``, having stopped it, should its stdout be over ``limit`` bytes. Should this be - cancelled (or fail), it is stopped too, rather than left writing to a + canceled (or fail), it is stopped too, rather than left writing to a pipe no one reads. ''' assert process.stdout is not None and process.stderr is not None diff --git a/dev_tools/pylibs/edgshell/__init__.py b/dev_tools/pylibs/edgshell/__init__.py index f423f1336a..99b3445ddd 100644 --- a/dev_tools/pylibs/edgshell/__init__.py +++ b/dev_tools/pylibs/edgshell/__init__.py @@ -18,16 +18,21 @@ import edgutil from pathlib import Path, PurePath -from typing import Any, List, Optional +from typing import Any, List, Optional, Set TOOL_PATH_GIT = edgutil.RequiredToolPath('git') -def read_process_output(args: List[str], *, cwd = None) -> str: - '''A simplified function for running a process and grabbing the output.''' +def read_process_output(args: List[str], *, cwd = None, + stderr = subprocess.STDOUT) -> str: + '''A simplified function for running a process and grabbing the output. + + By default, the output includes what the process writes to standard error. + Pass another value (e.g., subprocess.DEVNULL) to keep it out of the output + when the output is going to be parsed.''' completed_process = subprocess.run( args, stdout = subprocess.PIPE, - stderr = subprocess.STDOUT, + stderr = stderr, cwd = cwd, check = True ) @@ -343,6 +348,41 @@ def list_files_in_git_revision(commit: str, *, file_list = [f for f in file_list if len(f) > 0] return file_list +def list_binary_files_in_git_diff(revision_range: str, *, + cwd: Optional[Path] = None) -> Set[str]: + '''Return the paths git treats as binary in the diff of revision_range. + + Git decides by the content of the file (a NUL byte near its start) and by + the .gitattributes of its path. A file git treats as binary has "-" for + both of its counts in "git diff --numstat". + + This is a best effort to avoid checking files that have no text. If git + fails, return the empty set (quietly) rather than failing the caller. + ''' + try: + output = read_process_output( + [ + str(TOOL_PATH_GIT), + 'diff', + '--numstat', + '-z', + '--no-renames', + revision_range + ], + cwd = cwd, + # Diagnostics (e.g., about .gitattributes) would corrupt the records. + stderr = subprocess.DEVNULL + ) + except subprocess.CalledProcessError: + return set() + binary_files = set() + for entry in output.split('\0'): + added, _, rest = entry.partition('\t') + deleted, _, path = rest.partition('\t') + if added == '-' and deleted == '-' and path: + binary_files.add(path) + return binary_files + def list_tags_in_repo(*, cwd: Optional[Path] = None) -> List[str]: output = read_process_output( [ diff --git a/dev_tools/pylibs/edgspelling/__init__.py b/dev_tools/pylibs/edgspelling/__init__.py new file mode 100644 index 0000000000..bedc9f0bee --- /dev/null +++ b/dev_tools/pylibs/edgspelling/__init__.py @@ -0,0 +1,201 @@ +# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM +# Exceptions. +# See https://edgcpp.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +'''The hierarchical allowlist for the spell check. + +The list lives in dev_tools/spelling_allowlist.json as a tree of nodes. Each +node has these optional members: + + "words" a sorted list of lowercase words allowed with any capitalization + "names" a sorted list of words (proper nouns, say) allowed with exactly the + capitalization given + "paths" an object mapping a path component (a directory or file name) to a + child node + +For example: + + { + "words": ["lvalue"], + "names": ["EDG"], + "paths": { + "src": { + "words": ["demangle"], + "paths": {"templates.c": {"words": ["tokenize"]}} + } + } + } + +Each entry is a single word. The spell check breaks identifiers into words at +underscores and camel case boundaries before it looks at them, so an entry +like "foo_bar" or "fooBar" would never match anything, and the Allowlist +class rejects one. An identifier that preprocessing leaves whole, such as +"getXValue" (it has a one letter part), is a single word and is allowed. + +Lists are sorted by code point, without duplicates. The Allowlist class +validates the file when it is loaded, which fails if a list is out of order, a +word isn't lowercase, or a node is malformed. + +A file's allowed words are the union of those of every node on the way down to +it, so a word allowed for a directory is allowed for everything in it. +''' + +import json +import re + +import edgtools + +from pathlib import Path, PurePosixPath +from typing import Any, Dict, Match, Set + +DEFAULT_ALLOWLIST_PATH = edgtools.get_tools_dir() / 'spelling_allowlist.json' + +class AllowlistError(Exception): + '''The allowlist file is malformed.''' + +class Allowlist: + '''The allowlist file, loaded and validated. + + Construction reads the file and raises AllowlistError if it isn't valid + JSON or any node is malformed, out of order, or (for "words") not lowercase, + so a bad file is found up front. A missing file is an empty allowlist. + ''' + + def __init__(self, tree_path: Path = DEFAULT_ALLOWLIST_PATH) -> None: + self._tree_path = tree_path + self._tree: Dict[str, Any] = {} + if tree_path.is_file(): + self._tree = self._read() + self._validate() + + def words_for_path(self, repo_path: str) -> Set[str]: + '''Return the lowercase words allowed, in any case, for repo_path (repo + relative): the "words" of every node on the way down to it.''' + return self._entries_for_path(repo_path, 'words') + + def names_for_path(self, repo_path: str) -> Set[str]: + '''Return the names allowed, with exactly the capitalization given, for + repo_path (repo relative): the "names" of every node on the way down to + it.''' + return self._entries_for_path(repo_path, 'names') + + def _entries_for_path(self, repo_path: str, key: str) -> Set[str]: + node = self._tree + entries = set(node.get(key, [])) + for component in PurePosixPath(repo_path).parts: + node = node.get('paths', {}).get(component) + if node is None: + break + entries.update(node.get(key, [])) + return entries + + def _read(self) -> Dict[str, Any]: + try: + with open(self._tree_path, 'r', encoding = 'utf-8') as file_handle: + return json.load(file_handle) + except (OSError, ValueError) as error: + raise AllowlistError(f"{self._tree_path}: {error}") from error + + def _validate(self) -> None: + '''Check every node of the tree. + + A node must be an object with only "words", "names" and "paths", the + first two being sorted lists of strings (the words lowercase) and the last + an object of nodes. Raises AllowlistError for the first problem found. + ''' + pending = [(str(self._tree_path), self._tree)] + while pending: + where, node = pending.pop() + if not isinstance(node, dict): + raise AllowlistError(f"{where}: expected an object") + unknown_keys = set(node) - {'words', 'names', 'paths'} + if unknown_keys: + raise AllowlistError(f"{where}: unknown keys {sorted(unknown_keys)}") + for key in ('words', 'names'): + self._validate_entries(node.get(key, []), key, where) + children = node.get('paths', {}) + if not isinstance(children, dict): + raise AllowlistError(f"{where}: \"paths\" must be an object") + for name, child in children.items(): + pending.append((f"{where}: {name}", child)) + + @staticmethod + def _validate_entries(entries: Any, key: str, where: str) -> None: + '''Check that entries, the value of key in a node, is a list of strings + that is sorted, has no duplicates, is lowercase (for "words"), and is a + single word that preprocessing would leave whole.''' + if not isinstance(entries, list) or not all( + isinstance(entry, str) for entry in entries + ): + raise AllowlistError(f"{where}: \"{key}\" must be a list of strings") + for previous, entry in zip(entries, entries[1:]): + if entry == previous: + raise AllowlistError(f"{where}: \"{key}\" lists \"{entry}\" twice") + if entry < previous: + raise AllowlistError( + f"{where}: \"{key}\" is out of order: \"{entry}\" must come " + f"before \"{previous}\" (entries are sorted by code point)" + ) + if key == 'words': + for entry in entries: + if entry != entry.lower(): + raise AllowlistError( + f"{where}: \"words\" are matched ignoring case, so \"{entry}\" " + "must be lowercase (use \"names\" to require a capitalization)" + ) + for entry in entries: + if '_' in entry or preprocess(entry) != entry: + raise AllowlistError( + f"{where}: \"{key}\" entry \"{entry}\" would be split into " + "words before it is checked, so it could never match; list its " + "parts instead" + ) + +# A lowercase letter or digit followed by an uppercase letter ("fooBar"), or +# the last capital of an acronym followed by a capitalized word ("HTTPServer"). +_CAMEL_CASE_BOUNDARY_REGEX = re.compile( + r'(?<=[a-z0-9])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])' +) + +_TOKEN_REGEX = re.compile(r'[A-Za-z0-9]+') + +def _is_word_like(part: str) -> bool: + '''Return whether part could be a word or an acronym: at least two + characters, and either all capitals/digits or containing a vowel.''' + return len(part) >= 2 and ( + re.fullmatch(r'[A-Z0-9]+', part) is not None + or re.search(r'[aeiouyAEIOUY]', part) is not None + ) + +def _split_token(token_match: Match[str]) -> str: + token = token_match.group() + parts = _CAMEL_CASE_BOUNDARY_REGEX.split(token) + # Random looking data (base64, hashes) has one letter "humps", parts without + # vowels, and short parts on average, so it isn't split. Identifiers are: + # either their parts all look like words or acronyms ("setUp", "ABIs"), or + # they're long enough on average to be abbreviations like "Stmt" and "Msvc". + if (len(parts) > 1 + and min(len(part) for part in parts) >= 2 + and (all(_is_word_like(part) for part in parts) + or len(token) >= 3 * len(parts))): + return ' '.join(parts) + return token + +def _split_camel_case(text: str) -> str: + '''Return text with a space at each camel case boundary of its identifiers. + + The spell checker then sees "foo Bar" for "fooBar" (as it already sees "foo" + and "bar" for "foo_bar"), so identifiers needn't be allowed whole. A token + that looks like random data rather than an identifier, such as "getXValue" + or a base64 blob, is left whole. + ''' + return _TOKEN_REGEX.sub(_split_token, text) + +def preprocess(text: str) -> str: + '''Return text prepared for the spell checker. + + Identifiers are broken into the words they are made of, so that + "fooBar" is checked as "foo Bar" instead of being flagged whole. + ''' + return _split_camel_case(text) diff --git a/dev_tools/pylibs/edgspelling/py.typed b/dev_tools/pylibs/edgspelling/py.typed new file mode 100644 index 0000000000..e69de29bb2 diff --git a/dev_tools/pylibs/edgutil/__init__.py b/dev_tools/pylibs/edgutil/__init__.py index e2dba357ea..a3cb0458a9 100644 --- a/dev_tools/pylibs/edgutil/__init__.py +++ b/dev_tools/pylibs/edgutil/__init__.py @@ -450,7 +450,7 @@ def _active_asyncio_tasks( if not task.done() } -# The tasks ``_stop_asyncio_loop`` cancelled, which ``_finish_asyncio_tasks`` +# The tasks ``_stop_asyncio_loop`` canceled, which ``_finish_asyncio_tasks`` # doesn't cancel again: that would cut their cleanup short. _stopped_asyncio_tasks: 'weakref.WeakSet[asyncio.Task]' = weakref.WeakSet() @@ -463,10 +463,10 @@ def _stop_asyncio_loop(loop: asyncio.AbstractEventLoop) -> None: task.cancel() def _finish_asyncio_tasks(loop: asyncio.AbstractEventLoop) -> None: - '''Run ``loop`` until every task on it has finished, cancelling those + '''Run ``loop`` until every task on it has finished, canceling those ``_stop_asyncio_loop`` hasn't (e.g. those started as others cleaned up), as ``asyncio.run`` does; report what failed other than by being - cancelled. + canceled. ''' tasks = _active_asyncio_tasks(loop) while tasks: @@ -522,11 +522,11 @@ def run_asyncio_loop( task_consumer is a lambda function consuming the created loop and returning the initial async function asyncio. - If that task raises ``AsyncLoopFatalError``, remaining tasks are cancelled + If that task raises ``AsyncLoopFatalError``, remaining tasks are canceled the same way a SIGINT/SIGTERM would cancel them, and the error is re-raised to the caller. Either way, once stopped, the loop runs until - every task has finished (cancelling any started meanwhile), so each - cancelled task's cleanup completes. + every task has finished (canceling any started meanwhile), so each + canceled task's cleanup completes. Pass ``install_signal_handlers=False`` when the caller already registers process-level handlers and must not have them replaced by the event loop. @@ -563,7 +563,7 @@ async def async_terminate_within(process: asyncio.subprocess.Process, try: await asyncio.wait_for(process.wait(), allowance) except BaseException as ex: - # Out of time, or cancelled while waiting: kill it either way. + # Out of time, or canceled while waiting: kill it either way. with contextlib.suppress(ProcessLookupError): process.kill() if not isinstance(ex, asyncio.TimeoutError): diff --git a/dev_tools/services/acknowledg-forge/tests/test_client_cancellation.py b/dev_tools/services/acknowledg-forge/tests/test_client_cancellation.py index 7d54678fd3..e2c56939b6 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_client_cancellation.py +++ b/dev_tools/services/acknowledg-forge/tests/test_client_cancellation.py @@ -3,7 +3,7 @@ # See https://edgcpp.org/LICENSE.txt for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -'''Cancelling AcknowlEDG client requests undoes them, as failing does; +'''Canceling AcknowlEDG client requests undoes them, as failing does; losing the connection fails them. ''' @@ -200,7 +200,7 @@ async def test_subscribes_answered_as_cancelled_are_released() -> None: while len(connection.sent) == 0: await asyncio.sleep(0.01) - # Answered, then cancelled before it resumes to take the answer. + # Answered, then canceled before it resumes to take the answer. subscription_id = str(uuid.uuid4()) (future,) = session._pending_subscribe.values() future.set_result({**payload, 'subscription_id': subscription_id}) @@ -228,7 +228,7 @@ async def subscribe(payload: dict[str, Any]) -> uuid.UUID: return subscription_id session._subscribe = subscribe # type: ignore[method-assign] - # Cancelled while announcing the section, after it was subscribed. + # Canceled while announcing the section, after it was subscribed. adding = asyncio.create_task(review.add_section(_Handler())) # type: ignore await _cancel_once_sent(session, adding) assert review.sections() == [] diff --git a/dev_tools/services/acknowledg-forge/tests/test_lifecycle.py b/dev_tools/services/acknowledg-forge/tests/test_lifecycle.py index 559ac07f07..003bdef862 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_lifecycle.py +++ b/dev_tools/services/acknowledg-forge/tests/test_lifecycle.py @@ -67,7 +67,7 @@ async def test_motivating_example(tmp_path: Path, ] # The base worktree only has what's needed: the touched test, not its - # neighbours or the big unrelated directory. + # neighbors or the big unrelated directory. changes = harness.publisher.reviews[review_id][0] visible = (changes.root / 'visible.txt').read_text().split() assert 'tests/suite/a.sft.cpp' in visible diff --git a/dev_tools/services/acknowledg-forge/tests/test_pool.py b/dev_tools/services/acknowledg-forge/tests/test_pool.py index 3e629c79c0..2962ce57a9 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_pool.py +++ b/dev_tools/services/acknowledg-forge/tests/test_pool.py @@ -154,7 +154,7 @@ async def test_an_abandoned_requests_answer_is_dropped( make_pool: Any) -> None: pool = await make_pool(processes = 1) [pid] = pool.pids() - # Cancelled while its process works on it (as when its section is + # Canceled while its process works on it (as when its section is # removed): the process still answers it... medium = asyncio.create_task( pool.answer(env.section, _show_source('suite/medium.ifc')) diff --git a/dev_tools/spelling_allowlist.json b/dev_tools/spelling_allowlist.json new file mode 100644 index 0000000000..3899d26036 --- /dev/null +++ b/dev_tools/spelling_allowlist.json @@ -0,0 +1,746 @@ +{ + "words": [ + "accessor", + "accessors", + "activations", + "algorithmically", + "aliasee", + "allocator", + "allocators", + "allowlist", + "andes", + "ansi", + "atomics", + "awaitable", + "backtrace", + "bigint", + "bitfield", + "bitfields", + "bitmask", + "bitmasks", + "bitset", + "bitwidth", + "bitwise", + "bool", + "booleans", + "boxable", + "branchless", + "breakpoint", + "builtins", + "callee", + "canonicalize", + "canonicalized", + "cardinality", + "catenate", + "charset", + "codegen", + "compat", + "compilable", + "conditionalize", + "configurability", + "confusable", + "consteval", + "constexpr", + "constinit", + "constructible", + "copyable", + "coroutine", + "coroutine's", + "coroutines", + "corrigendum", + "cygwin", + "dealias", + "dealiasable", + "deallocate", + "deallocated", + "deallocates", + "deallocation", + "deallocations", + "debuggable", + "declarator", + "declarators", + "decltype", + "decodable", + "deduplicate", + "defeatable", + "demangle", + "demangled", + "demangler", + "demangles", + "demangling", + "denorm", + "dereference", + "dereferenced", + "dereferences", + "dereferencing", + "deserialization", + "designator", + "designators", + "destructions", + "destructor", + "destructor's", + "destructors", + "dev", + "dir", + "disambiguating", + "emacs", + "encodings", + "endian", + "endianness", + "enum", + "executables", + "expander", + "extensionless", + "extern", + "fallthrough", + "filesystem", + "finalizer", + "finalizers", + "fixups", + "flakey", + "foldable", + "forall", + "formatter", + "fortran", + "glvalue", + "glvalues", + "glyphs", + "goto", + "gotos", + "inclass", + "indirecting", + "indirections", + "initializations", + "initializer", + "initializers", + "inlinable", + "inlined", + "inliner", + "inlines", + "inlining", + "instantiatable", + "instantiation", + "instantiations", + "interconvertible", + "interpolator", + "intrinsics", + "introducer", + "invocable", + "itanium", + "json", + "lexically", + "lexing", + "lifecycle", + "linkers", + "linux", + "lookahead", + "lookups", + "lossy", + "lvalue", + "lvalues", + "macos", + "manglings", + "matcher", + "memberwise", + "memoized", + "metaprogramming", + "microsoft", + "misparsed", + "monadic", + "multibyte", + "multicast", + "multicharacter", + "multiline", + "multithreaded", + "multiversion", + "multiversioning", + "mutator", + "namespace", + "namespaces", + "natively", + "nestable", + "niladic", + "noexcept", + "nonblank", + "nonblock", + "nonclass", + "noncomment", + "nonconstant", + "nondeclarative", + "nondeduced", + "nondefining", + "nondefinition", + "nondelegating", + "nondeleted", + "nondependent", + "nondiscretionary", + "nonexplicit", + "nonexternal", + "nonfunction", + "nongeneric", + "noninline", + "nonliteral", + "nonlocal", + "nonmanaged", + "nonnegative", + "nonnested", + "nonnull", + "nonoverriding", + "nonpointer", + "nonreal", + "nonregister", + "nonshared", + "nonspecialized", + "nonstatic", + "nonstd", + "nonstrict", + "nontemplate", + "nontentative", + "nontrivially", + "nontype", + "nonvirtual", + "noop", + "nullability", + "nullable", + "objectless", + "optimizers", + "origined", + "overaligned", + "overloadable", + "overrider", + "oversized", + "paren", + "parens", + "parentless", + "pathname", + "performant", + "pointee", + "posix", + "postfix", + "postincrement", + "pragma", + "pragma's", + "pragmas", + "preallocated", + "preallocation", + "precedences", + "prechecked", + "precompilation", + "precompiled", + "predeclare", + "predeclared", + "predeclaring", + "predefine", + "predefines", + "preinclude", + "preincludes", + "prelink", + "prelinker", + "prelinking", + "preload", + "prelower", + "prelowering", + "preorder", + "prepended", + "prepending", + "prepends", + "preprocess", + "preprocessed", + "preprocessing", + "preprocessor", + "prescan", + "prescanned", + "prescanning", + "prototyped", + "prvalue", + "prvalues", + "pseudocode", + "punctuator", + "qualifiable", + "radix", + "readme", + "recompilations", + "recurse", + "recurses", + "redeclarable", + "redeclaration", + "redeclarations", + "redeclare", + "redeclared", + "redeclares", + "redeclaring", + "redefinitions", + "redistributable", + "reencoded", + "referenceable", + "relink", + "remappings", + "repositioned", + "representable", + "representationally", + "repush", + "rescan", + "rescannable", + "rescanned", + "rescanning", + "rescans", + "resizable", + "rethrow", + "rethrowing", + "rethrown", + "rethrows", + "rvalue", + "rvalues", + "sanitization", + "scalable", + "semivisible", + "severities", + "signedness", + "sizeless", + "solaris", + "sparc", + "src", + "stringize", + "stringized", + "struct", + "structs", + "subaggregate", + "subblocks", + "subcases", + "subdirectories", + "subdirectory", + "subexpressions", + "subfield", + "subfields", + "subkind", + "sublist", + "subobject", + "subobjects", + "subrange", + "subscopes", + "subscriptable", + "subscripted", + "subscripting", + "subsequence", + "substitutable", + "substring", + "substrings", + "subtree", + "subtrees", + "sunos", + "superset", + "symlink", + "tagless", + "tangut", + "templated", + "tokenization", + "tokenize", + "tokenized", + "traceback", + "tradeoff", + "triadic", + "trigraph", + "trigraphs", + "typedef", + "typedefs", + "typeinfo", + "typename", + "typeref", + "typerefs", + "unaliased", + "unapplied", + "unbundle", + "unbundling", + "uncallable", + "unclosed", + "uncompress", + "undefine", + "undefinition", + "undefinitions", + "unescaped", + "unevaluated", + "unexpanded", + "unhandled", + "unhidden", + "unhiding", + "unicode", + "uninstantiated", + "unix", + "unlink", + "unlinked", + "unlowered", + "unmangled", + "unparenthesized", + "unparsed", + "unprototyped", + "unreferenced", + "unrepresentable", + "unresolvable", + "unscan", + "unscanned", + "unscoped", + "unsequenced", + "unshared", + "unspecialized", + "unterminated", + "untracked", + "utf", + "validator", + "variadic", + "virtuality", + "vtable", + "vtables", + "whitespace", + "xvalue", + "xvalues", + "yaml" + ], + "names": [ + "EDG", + "LLVM", + "SPDX" + ], + "paths": { + ".github": { + "words": [ + "checkboxes", + "dropdown" + ] + }, + "bases": { + "paths": { + "cmake-native": { + "words": [ + "unretained" + ] + }, + "docker": { + "paths": { + "dev-env": { + "words": [ + "cobol", + "multilib" + ] + } + } + } + } + }, + "benchmarks": { + "paths": { + "baselines": { + "paths": { + "project": { + "words": [ + "deregister", + "localtime", + "mutex", + "tunable", + "tunables", + "unsave" + ] + } + } + } + } + }, + "dev_tools": { + "words": [ + "ascii", + "backend", + "backoff", + "bitshift", + "buildable", + "bytearray", + "cachegrind", + "callout", + "commenter", + "deduplicated", + "delist", + "deserialize", + "edgacknowledg", + "edgbench", + "edgchanges", + "edgcppgen", + "edgdocker", + "edggpp", + "edgifc", + "edgpack", + "edgshell", + "edgspelling", + "edgtest", + "edgtools", + "edgutil", + "fullmatch", + "gzip", + "hostname", + "inbounds", + "ingestor", + "init", + "isinstance", + "iterable", + "len", + "localhost", + "loopback", + "metadatas", + "metavar", + "monospace", + "normalizer", + "parseable", + "pathlib", + "poller", + "postgresql", + "prebuild", + "reflectable", + "rematerialize", + "repo", + "sandboxing", + "serializer", + "splitable", + "splittable", + "staticmethod", + "str", + "symlinks", + "uncomment", + "unsandboxed", + "untrusted", + "untyped", + "useradd", + "workspaces", + "worktree", + "worktrees" + ], + "paths": { + "bin": { + "words": [ + "commandline", + "deduplication", + "subparsers" + ] + }, + "docker": { + "words": [ + "centos", + "frontends" + ] + }, + "pylibs": { + "words": [ + "unregister" + ], + "paths": { + "edgacknowledg": { + "words": [ + "sandboxed", + "sqlite" + ] + }, + "edgifc": { + "words": [ + "unmanaged" + ] + }, + "edgtest": { + "words": [ + "dedupe" + ] + } + } + }, + "services": { + "words": [ + "decrypts", + "jinja", + "sudo", + "unencrypted" + ], + "paths": { + "acknowledg": { + "words": [ + "focusable", + "lowercasing", + "scrollable", + "viewport" + ] + }, + "acknowledg-forge": { + "words": [ + "decrypted", + "frontend", + "pluggy" + ] + } + } + } + } + }, + "doc": { + "paths": { + "source": { + "words": [ + "latin", + "subcase" + ] + } + } + }, + "src": { + "words": [ + "aggregatable", + "amongst", + "baseward", + "bigints", + "bindable", + "bottommost", + "callability", + "callee's", + "callees", + "checksums", + "cloneable", + "completable", + "conformant", + "constness", + "copyability", + "copysign", + "dealiased", + "dealiasing", + "deallocating", + "deducibility", + "deleter", + "denormalize", + "devirtualization", + "dimensioned", + "disambiguated", + "disambiguates", + "disambiguator", + "disjunction", + "disjunctions", + "earlyclobber", + "earlyclobbers", + "elementwise", + "enum's", + "externs", + "glvalueness", + "growable", + "highlightable", + "immediates", + "incrementation", + "indexable", + "initializable", + "initializer's", + "instantiation's", + "interconvertibility", + "interpolators", + "linkable", + "lvalueness", + "memoize", + "metafunctions", + "misdeclared", + "mistyped", + "multiversioned", + "namespace's", + "nonaggregate", + "nonautonomous", + "noncanonical", + "nonconstants", + "nonoverloaded", + "nonpolymorphic", + "nonpositive", + "nonprimary", + "nonprintable", + "nonprototype", + "nonprototyped", + "nonqualified", + "nonreference", + "nonscalar", + "nonstring", + "nontag", + "nontags", + "nontemplates", + "nontrailing", + "nontypes", + "optimizable", + "overriders", + "parameterization", + "parenthesization", + "patchable", + "popcount", + "postorder", + "preallocate", + "precisions", + "predeclarator", + "preincluded", + "prelowered", + "preprocessors", + "prescans", + "priori", + "programmatically", + "promotable", + "punctuators", + "reachability", + "reactivations", + "reallocations", + "recomputation", + "recursing", + "refetch", + "reinitialization", + "relinked", + "reparse", + "reparsing", + "resumable", + "rewritings", + "rvalued", + "selectable", + "stringizing", + "struct's", + "subarray", + "subblock", + "subexpression", + "sublist's", + "sublists", + "subnodes", + "subsequences", + "substatement", + "substatements", + "subtypes", + "sunpro", + "tokenizing", + "typedef's", + "typeinfos", + "unallocated", + "unassociated", + "unbuffered", + "uncoalesced", + "uncompilable", + "uncompletable", + "undeduced", + "undefines", + "undefining", + "underflowed", + "unelided", + "unhide", + "unlinking", + "unmapped", + "unmark", + "unoptimized", + "unprefixed", + "unpromoted", + "unselected", + "unsubstituted", + "unsuffixed", + "unvisited", + "unwidened", + "virtualness" + ], + "names": [ + "Coverity", + "Gimpel" + ] + }, + "util": { + "words": [ + "recompiles" + ] + } + } +} diff --git a/dev_tools/tests/test_edg_check_file_changes.py b/dev_tools/tests/test_edg_check_file_changes.py new file mode 100644 index 0000000000..6025bff2cb --- /dev/null +++ b/dev_tools/tests/test_edg_check_file_changes.py @@ -0,0 +1,433 @@ +# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM +# Exceptions. +# See https://edgcpp.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +'''Tests for the line length exemptions of edg-check-file-changes and for the +hierarchical spelling allowlist (edgspelling). + +Run with: python3 -m unittest discover -s dev_tools/tests +(with dev_tools/pylibs on PYTHONPATH). +''' + +import json +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +import edgspelling +import edgshell + +from tool_tree import PYLIBS_DIR, make_tool_tree + +from pathlib import Path + +DEV_TOOLS_DIR = Path(__file__).resolve().parents[1] +CHECK_TOOL_PATH = DEV_TOOLS_DIR / 'bin' / 'edg-check-file-changes' + +LONG_LINE = 'x' * 100 + '\n' + +def _run_check(tmp_dir: Path, name: str, content) -> dict: + '''Run the tool on a new file called name; return its result entry.''' + file_path = tmp_dir / name + if isinstance(content, bytes): + file_path.write_bytes(content) + else: + file_path.write_text(content) + # The old version is empty, so everything in the new file is new. + old_path = tmp_dir / ('old_' + name) + old_path.write_text('') + input_path = tmp_dir / 'input.json' + output_path = tmp_dir / 'output.json' + input_path.write_text(json.dumps({name: [str(old_path), str(file_path)]})) + # An empty allowlist keeps the result independent of the shipped one. + pylibs_dir = make_tool_tree(tmp_dir, {}) + subprocess.run( + [sys.executable, str(CHECK_TOOL_PATH), str(input_path), str(output_path)], + check = True, + cwd = '/', + env = {**os.environ, 'PYTHONPATH': str(pylibs_dir)} + ) + return json.loads(output_path.read_text())[name] + +def _run_tool(tmp_dir: Path, name: str, content: str, *, + pylibs_dir: Path = PYLIBS_DIR) -> subprocess.CompletedProcess: + '''Run the tool on a new file called name, with pylibs_dir as its + packages; the result entry is in tmp_dir/output.json if it succeeded.''' + file_path = tmp_dir / name + file_path.write_text(content) + # The old version is empty, so everything in the new file is new. + old_path = tmp_dir / ('old_' + name) + old_path.write_text('') + input_path = tmp_dir / 'input.json' + input_path.write_text(json.dumps({name: [str(old_path), str(file_path)]})) + return subprocess.run( + [sys.executable, str(CHECK_TOOL_PATH), str(input_path), + str(tmp_dir / 'output.json')], + cwd = '/', + env = {**os.environ, 'PYTHONPATH': str(pylibs_dir)}, + capture_output = True, + text = True + ) + +@unittest.skipUnless(shutil.which('aspell'), 'aspell not installed') +class ToolAllowlistTests(unittest.TestCase): + '''The tool's use of the allowlist: words ignore case, names don't.''' + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.tmp_dir = Path(self._tmp.name) + + def _misspellings(self, allowlist, content: str): + pylibs_dir = make_tool_tree(self.tmp_dir, allowlist) + result = _run_tool( + self.tmp_dir, 'a.md', content, pylibs_dir = pylibs_dir + ) + self.assertEqual(result.returncode, 0, result.stderr) + output = json.loads((self.tmp_dir / 'output.json').read_text()) + return output['a.md']['misspellings'] + + def test_unlisted_words_are_reported(self) -> None: + self.assertEqual( + self._misspellings({}, 'zxqvortle\n'), ['zxqvortle'] + ) + + def test_words_match_in_any_case(self) -> None: + self.assertEqual( + self._misspellings( + {'words': ['zxqvortle']}, 'zxqvortle Zxqvortle ZXQVORTLE\n' + ), + [] + ) + + def test_names_match_exactly(self) -> None: + self.assertEqual( + self._misspellings( + {'names': ['Zxqvortle']}, 'Zxqvortle zxqvortle ZXQVORTLE\n' + ), + ['ZXQVORTLE', 'zxqvortle'] + ) + + def test_refuses_to_start_with_a_bad_allowlist(self) -> None: + for allowlist in ( + '{"words": ["b", "a"]}', + '{"words": ["A"]}', + '{"names": ["b", "A"]}', + '{', + ): + with self.subTest(allowlist = allowlist), \ + tempfile.TemporaryDirectory() as tmp_dir_str: + tmp_dir = Path(tmp_dir_str) + pylibs_dir = make_tool_tree(tmp_dir, allowlist) + result = _run_tool(tmp_dir, 'a.md', 'hi\n', pylibs_dir = pylibs_dir) + self.assertEqual(result.returncode, 1) + self.assertIn('Invalid spelling allowlist', result.stderr) + self.assertFalse((tmp_dir / 'output.json').exists()) + +class OverlongLineTests(unittest.TestCase): + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.tmp_dir = Path(self._tmp.name) + + def test_data_files_are_exempt(self) -> None: + for name in ('a.json', 'a.yml', 'a.yaml'): + with self.subTest(name = name): + result = _run_check(self.tmp_dir, name, LONG_LINE) + self.assertEqual(len(result['overlong_lines']), 0) + + def test_other_files_are_still_checked(self) -> None: + for name in ('a.md', 'a.py'): + with self.subTest(name = name): + result = _run_check(self.tmp_dir, name, LONG_LINE) + self.assertEqual(len(result['overlong_lines']), 1) + + def test_md_tables_are_exempt(self) -> None: + content = MD_TABLE + 'Text.\n' + result = _run_check(self.tmp_dir, 'a.md', content) + self.assertEqual(result['overlong_lines'], {}) + + def test_md_text_after_a_table_is_checked(self) -> None: + content = MD_TABLE + LONG_LINE + result = _run_check(self.tmp_dir, 'a.md', content) + self.assertEqual(list(result['overlong_lines']), ['4']) + _, context = result['overlong_lines']['4'] + # The table is still shown as context. + self.assertEqual(context, MD_TABLE.splitlines(True)[-1:] + [LONG_LINE]) + + def test_md_pipe_lines_without_a_delimiter_row_are_checked(self) -> None: + result = _run_check(self.tmp_dir, 'a.md', '| ' + LONG_LINE) + self.assertEqual(list(result['overlong_lines']), ['1']) + + def test_rst_grid_tables_are_exempt(self) -> None: + content = RST_GRID_TABLE + '\nText.\n' + result = _run_check(self.tmp_dir, 'a.rst', content) + self.assertEqual(result['overlong_lines'], {}) + + def test_rst_simple_tables_are_exempt(self) -> None: + content = 'Title\n=====\n\n' + RST_SIMPLE_TABLE + '\n' + LONG_LINE + result = _run_check(self.tmp_dir, 'a.rst', content) + self.assertEqual(list(result['overlong_lines']), ['10']) + + def test_rst_line_blocks_are_exempt(self) -> None: + content = ( + 'Text.\n' + '\n' + ' | ' + LONG_LINE + + ' |\n' + ' | Short.\n' + ' ' + LONG_LINE + + '\n' + + LONG_LINE + ) + result = _run_check(self.tmp_dir, 'a.rst', content) + self.assertEqual(list(result['overlong_lines']), ['8']) + + def test_rst_pipes_without_a_space_are_checked(self) -> None: + result = _run_check(self.tmp_dir, 'a.rst', '|' + LONG_LINE) + self.assertEqual(list(result['overlong_lines']), ['1']) + + def test_tables_in_other_files_are_checked(self) -> None: + result = _run_check(self.tmp_dir, 'a.py', MD_TABLE + RST_GRID_TABLE) + self.assertEqual(len(result['overlong_lines']), 2) + +LONG_CELL = 'y' * 90 + +MD_TABLE = ( + '| Name | Description |\n' + '| --- | :-: |\n' + f"| a | {LONG_CELL} |\n" +) + +RST_GRID_TABLE = ( + '+------+-------------+\n' + '| Name | Description |\n' + '+======+=============+\n' + f"| a | {LONG_CELL} |\n" + '+------+-------------+\n' +) + +RST_SIMPLE_TABLE = ( + '==== ===========\n' + 'Name Description\n' + '==== ===========\n' + f"a {LONG_CELL}\n" + '==== ===========\n' +) + +class BinaryFilesInDiffTests(unittest.TestCase): + '''The pre-push hook skips the files git says are binary.''' + + @unittest.skipUnless(shutil.which('git'), 'git not installed') + def test_matches_git(self) -> None: + with tempfile.TemporaryDirectory() as tmp_dir_str: + repo = Path(tmp_dir_str) + + def git(*args: str) -> str: + return subprocess.run( + ['git', '-c', 'user.name=t', '-c', 'user.email=t@example.com', + '-c', 'commit.gpgsign=false', *args], + cwd = repo, check = True, capture_output = True, text = True + ).stdout + + git('init', '-q') + (repo / 'base.txt').write_text('base\n') + # Git warns about the negative pattern each time it reads this file. + # The warning is written before the first record of the numstat output + # (attr.dat, below), so it must not become part of that record. + (repo / '.gitattributes').write_text('attr.dat binary\n!*.x binary\n') + git('add', '.') + git('commit', '-q', '--no-verify', '-m', 'base') + (repo / 'text.md').write_text('words\n') + (repo / 'data.bin').write_bytes(b'ab\0cd') + (repo / 'héllo.bin').write_bytes(b'\0\0') + (repo / 'attr.dat').write_text('looks like text\n') + git('add', '.') + git('commit', '-q', '--no-verify', '-m', 'new') + self.assertEqual( + edgshell.list_binary_files_in_git_diff('HEAD~1..HEAD', cwd = repo), + {'data.bin', 'héllo.bin', 'attr.dat'} + ) + + @unittest.skipUnless(shutil.which('git'), 'git not installed') + def test_git_failure_is_not_fatal(self) -> None: + with tempfile.TemporaryDirectory() as tmp_dir_str: + repo = Path(tmp_dir_str) + subprocess.run(['git', 'init', '-q'], cwd = repo, check = True) + self.assertEqual( + edgshell.list_binary_files_in_git_diff('no-such-rev..HEAD', + cwd = repo), + set() + ) + +class AllowlistTests(unittest.TestCase): + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.tree_path = Path(self._tmp.name) / 'words.json' + + def _write(self, tree) -> None: + self.tree_path.write_text( + tree if isinstance(tree, str) else json.dumps(tree) + ) + + def _words(self, repo_path: str): + return edgspelling.Allowlist(self.tree_path).words_for_path(repo_path) + + def _names(self, repo_path: str): + return edgspelling.Allowlist(self.tree_path).names_for_path(repo_path) + + def test_union_down_the_tree(self) -> None: + self._write({ + 'words': ['global'], + 'names': ['Root'], + 'paths': { + 'src': { + 'words': ['dirword'], + 'paths': {'a.c': {'words': ['fileword'], 'names': ['File']}} + }, + 'doc': {'words': ['docword']} + } + }) + self.assertEqual(self._words('README.md'), {'global'}) + self.assertEqual(self._words('src/b.c'), {'global', 'dirword'}) + self.assertEqual( + self._words('src/a.c'), {'global', 'dirword', 'fileword'} + ) + self.assertEqual(self._words('doc/x/y.rst'), {'global', 'docword'}) + self.assertEqual(self._names('README.md'), {'Root'}) + self.assertEqual(self._names('src/b.c'), {'Root'}) + self.assertEqual(self._names('src/a.c'), {'Root', 'File'}) + + def test_names_are_per_path(self) -> None: + self._write({'paths': {'src': {'names': ['Bjarne']}}}) + self.assertEqual(self._names('src/a.c'), {'Bjarne'}) + self.assertEqual(self._names('doc/a.rst'), set()) + + def test_path_names_do_not_collide_with_keys(self) -> None: + self._write({'paths': {'words': {'words': ['inner']}}}) + self.assertEqual(self._words('words/f.txt'), {'inner'}) + self.assertEqual(self._words('f.txt'), set()) + + def test_missing_file_allows_nothing(self) -> None: + self.assertEqual(self._words('src/a.c'), set()) + + def test_malformed_files_raise(self) -> None: + for content in ( + '{', + '[]', + '{"words": "x"}', + '{"words": [1]}', + '{"names": "x"}', + '{"words": ["Mixed"]}', + '{"paths": []}', + '{"other": 1}', + '{"paths": {"src": {"wrods": []}}}', + # Out of order, or duplicated, at the root and deeper in the tree. + '{"words": ["beta", "alpha"]}', + '{"names": ["Beta", "Alpha"]}', + '{"words": ["alpha", "alpha"]}', + '{"names": ["Alpha", "Alpha"]}', + '{"paths": {"src": {"paths": {"a.c": {"words": ["b", "a"]}}}}}', + # Names sort by code point, so a capital comes before a lowercase. + '{"names": ["alpha", "Beta"]}', + ): + with self.subTest(content = content): + self._write(content) + with self.assertRaises(edgspelling.AllowlistError): + self._words('src/a.c') + + def test_sorted_lists_load(self) -> None: + self._write({ + 'words': ['alpha', 'beta', 'gamma'], + 'names': ['Alpha', 'Beta', 'alpha'], + }) + edgspelling.Allowlist(self.tree_path) + + def test_errors_name_the_problem(self) -> None: + for content, expected in ( + ('{"words": ["b", "a"]}', 'out of order'), + ('{"words": ["a", "a"]}', 'twice'), + ('{"words": ["A"]}', 'lowercase'), + ('{"paths": {"src": {"words": ["b", "a"]}}}', ': src'), + ): + with self.subTest(content = content): + self._write(content) + with self.assertRaisesRegex(edgspelling.AllowlistError, expected): + edgspelling.Allowlist(self.tree_path) + + def test_bad_file_is_refused_when_loaded(self) -> None: + # Even a path no bad node is on fails: the whole file is validated. + self._write({'paths': {'src': {'words': ['b', 'a']}}}) + with self.assertRaises(edgspelling.AllowlistError): + self._words('doc/a.rst') + +class PreprocessTests(unittest.TestCase): + def test_splits_camel_case_identifiers(self) -> None: + for text, expected in ( + ('fooBar', 'foo Bar'), + ('FooBarBaz', 'Foo Bar Baz'), + # Abbreviations without vowels are fine as parts. + ('StmtBlock', 'Stmt Block'), + ('setUp', 'set Up'), + ('ABIs', 'AB Is'), + ('WinCE', 'Win CE'), + ('TraitMsvcDeclAttrs', 'Trait Msvc Decl Attrs'), + ('HTTPServer', 'HTTP Server'), + ('parseJSON2Text', 'parse JSON2 Text'), + ('foo_barBaz', 'foo_bar Baz'), + ('see fooBar, then BazQux.', 'see foo Bar, then Baz Qux.'), + ): + with self.subTest(text = text): + self.assertEqual(edgspelling.preprocess(text), expected) + + def test_leaves_other_tokens_whole(self) -> None: + for text in ( + 'plain', 'Capitalized', 'ALLCAPS', 'snake_case', + # One letter parts, or short parts on average: random looking data. + 'getXValue', 'QaBcDeFgH', 'aBcd', 'xBqzK', 'QaBcDeFgHiJk', + ): + with self.subTest(text = text): + self.assertEqual(edgspelling.preprocess(text), text) + +class AllowlistSplitEntryTests(unittest.TestCase): + def _load(self, tree) -> None: + with tempfile.TemporaryDirectory() as tmp_dir_str: + path = Path(tmp_dir_str) / 'allowlist.json' + path.write_text(json.dumps(tree), encoding = 'utf-8') + edgspelling.Allowlist(path) + + def test_rejects_entries_that_preprocessing_splits(self) -> None: + # "words" must be lowercase, so only an underscore can split one. + for key, entry in ( + ('words', 'foo_bar'), + ('names', 'fooBar'), ('names', 'HTTPServer'), ('names', 'foo_bar'), + ): + with self.subTest(key = key, entry = entry): + with self.assertRaisesRegex( + edgspelling.AllowlistError, 'would be split' + ): + self._load({key: [entry]}) + + def test_accepts_identifiers_preprocessing_leaves_whole(self) -> None: + self._load({'names': ['getXValue'], 'words': ['plain']}) + +class ShippedAllowlistTests(unittest.TestCase): + def test_shipped_file_is_well_formed(self) -> None: + # Raises AllowlistError if the file is malformed, a list is out of order, + # or a word isn't lowercase, so a bad change to it fails CI. + edgspelling.Allowlist() + +@unittest.skipUnless(shutil.which('aspell'), 'aspell not installed') +class SpellingIntegrationTests(unittest.TestCase): + def test_unknown_word_is_reported(self) -> None: + with tempfile.TemporaryDirectory() as tmp_dir_str: + result = _run_check(Path(tmp_dir_str), 'a.md', 'zxqvortle\n') + self.assertIn('zxqvortle', result['misspellings']) + +if __name__ == '__main__': + unittest.main() diff --git a/dev_tools/tests/test_edg_collect_changes.py b/dev_tools/tests/test_edg_collect_changes.py index c22b0988f1..433b8ecd30 100644 --- a/dev_tools/tests/test_edg_collect_changes.py +++ b/dev_tools/tests/test_edg_collect_changes.py @@ -27,6 +27,8 @@ import edgchanges import edgutil +from tool_tree import make_tool_tree + from pathlib import Path from typing import Any, Dict @@ -544,6 +546,8 @@ def _check(self, repo_path: str, text: str) -> Dict[str, Any]: input_file.write_text( json.dumps({repo_path: [str(old_file), str(new_file)]}) ) + # An empty allowlist keeps the result independent of the shipped one. + pylibs_dir = make_tool_tree(self.tmp_dir, {}) # The checker looks for the entry's Changes file relative to its working # directory. subprocess.run( @@ -551,7 +555,7 @@ def _check(self, repo_path: str, text: str) -> Dict[str, Any]: str(output_file)], check = True, cwd = str(self.tmp_dir), - env = dict(os.environ, PYTHONPATH = str(DEV_TOOLS_DIR / 'pylibs')) + env = dict(os.environ, PYTHONPATH = str(pylibs_dir)) ) return json.loads(output_file.read_text())[repo_path] diff --git a/dev_tools/tests/test_edgutil.py b/dev_tools/tests/test_edgutil.py index 3237da7bd8..8fc833e8d4 100644 --- a/dev_tools/tests/test_edgutil.py +++ b/dev_tools/tests/test_edgutil.py @@ -87,7 +87,7 @@ async def consumer(loop): try: await asyncio.sleep(100) except asyncio.CancelledError: - # Starting a task while stopping: it's cancelled, and awaited, too. + # Starting a task while stopping: it's canceled, and awaited, too. loop.create_task(clean_up_slowly('latecomer')) if scenario == 'sigint': raise diff --git a/dev_tools/tests/tool_tree.py b/dev_tools/tests/tool_tree.py new file mode 100644 index 0000000000..cf8078f70c --- /dev/null +++ b/dev_tools/tests/tool_tree.py @@ -0,0 +1,38 @@ +# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM +# Exceptions. +# See https://edgcpp.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +'''Helper for tests that run edg-check-file-changes: gives it a private copy +of its packages beside an allowlist of the test's choosing, so the results +don't depend on the production spelling allowlist.''' + +import json +import shutil + +from pathlib import Path +from typing import Any + +PYLIBS_DIR = Path(__file__).resolve().parents[1] / 'pylibs' + +# The pylibs packages the tool imports. +TOOL_PYLIBS = ('edgchanges', 'edgspelling', 'edgtools', 'edgutil') + +def make_tool_tree(tmp_dir: Path, allowlist: Any) -> Path: + '''Copy the packages the tool needs into tmp_dir/dev_tools/pylibs, beside + an allowlist file holding allowlist (JSON text, or a value to dump). The + tool finds its allowlist relative to the packages, so this gives it one of + our own. Packages already copied are kept; the allowlist is rewritten. + Return the pylibs directory.''' + pylibs_dir = tmp_dir / 'dev_tools' / 'pylibs' + for package in TOOL_PYLIBS: + if not (pylibs_dir / package).exists(): + shutil.copytree( + PYLIBS_DIR / package, + pylibs_dir / package, + ignore = shutil.ignore_patterns('__pycache__') + ) + (tmp_dir / 'dev_tools' / 'spelling_allowlist.json').write_text( + allowlist if isinstance(allowlist, str) else json.dumps(allowlist) + ) + return pylibs_dir diff --git a/dev_tools/typecheck-python.sh b/dev_tools/typecheck-python.sh index 61eb1417a1..aec03feb44 100755 --- a/dev_tools/typecheck-python.sh +++ b/dev_tools/typecheck-python.sh @@ -122,6 +122,9 @@ pushd pylibs/ > /dev/null pushd edgshell/ > /dev/null check_types __init__.py popd > /dev/null + pushd edgspelling/ > /dev/null + check_types_strict __init__.py + popd > /dev/null pushd edgtest/ > /dev/null check_types_strict __init__.py pushd inputs/ > /dev/null diff --git a/doc/source/exceptions.rst b/doc/source/exceptions.rst index 5178d16806..bc8af1e5dc 100644 --- a/doc/source/exceptions.rst +++ b/doc/source/exceptions.rst @@ -293,7 +293,7 @@ The function clean-up entry contains A region is associated with a specific sequence of clean-up actions that would be required should an exception occur. A new region is typically started -whenever a destructable object comes into or goes out of scope. +whenever a destructible object comes into or goes out of scope. The global variable ``__eh_curr_region`` records the region number currently being executed. It is saved each time a new function entry or ``try`` block is diff --git a/doc/source/lex_pp.rst b/doc/source/lex_pp.rst index 9c10a0d78d..8009ceab0c 100644 --- a/doc/source/lex_pp.rst +++ b/doc/source/lex_pp.rst @@ -466,7 +466,7 @@ stack. Reusable and nonreusable token caches may be freely intermixed; template instantiations may occur while scanning tokens from a cache and a template instantiation will usually create new caches, as needed, when doing -look-aheads, as described earlier. +look-ahead, as described earlier. Source Positions ---------------- diff --git a/include_c++/Changes b/include_c++/Changes index 0360f1efc8..98f4aade1f 100644 --- a/include_c++/Changes +++ b/include_c++/Changes @@ -439,7 +439,7 @@ added in version 2.31, but were disabled because the front end did not yet support placement delete or overloaded delete operators. ------------------------------------------------------------------------------- -Version 2.33, Octover 2, 1996 +Version 2.33, October 2, 1996 9/25/96 uncaught_exception has been added to exception.h diff --git a/lib_src/eh.h b/lib_src/eh.h index 4ab81b84c4..9d7ddafc31 100644 --- a/lib_src/eh.h +++ b/lib_src/eh.h @@ -177,7 +177,7 @@ typedef struct an_eh_array_supplement { } an_eh_array_supplement; -/* Structure that describes a single destructable object. */ +/* Structure that describes a single destructible object. */ typedef struct an_eh_region_descr *an_eh_region_descr_ptr; typedef struct an_eh_region_descr { a_void_function_ptr diff --git a/src/Changes b/src/Changes index f8c9d33a98..6586d598e4 100644 --- a/src/Changes +++ b/src/Changes @@ -19040,7 +19040,7 @@ Hang on GCC target pragma with multiple string arguments Previously, passing multiple string arguments to the GCC target pragma had caused the front end to hang. A change has been made to enable string literal -concatentation when parsing GCC pragmas so the following is now accepted in +concatenation when parsing GCC pragmas so the following is now accepted in GNU emulation modes: _Pragma("GCC push_options") @@ -25262,14 +25262,14 @@ Consider: template X &operator<<(const T &); }; template - concept Outable = requires(U &s, T const &x) { s << x; }; + concept Writable = requires(U &s, T const &x) { s << x; }; template - void operator<<(X &x, const T &o) requires (!Outable) { + void operator<<(X &x, const T &o) requires (!Writable) { x << 0; // Error now caught earlier. } This example is ill-formed because the constraint that must be checked for -the expression x << 0 depends on itself (via the concept Outable, which will +the expression x << 0 depends on itself (via the concept Writable, which will recursively substitute the non-member operator<< candidate). Previously, the front end caught this via a general mechanism that limits the number of recursive substitutions. If the limit is set too high, however, that can @@ -31646,7 +31646,7 @@ list. This is now fixed. For example, with --g++: 6/2/21 [EDGcpfe/24389] -Spurious consteval failure in temmplated contexts +Spurious consteval failure in templated contexts Consider: @@ -35283,7 +35283,7 @@ EDGcpfe/15168,EDGcpfe/20235) were enabled in clang mode were not correct. As noted in the original description, the feature was supported only in C++14 and newer modes and when gnu_version was at least 40900. In clang mode, the restriction to C++14 mode was incorrect, and the dependency on -gnu_version was unobvious. The front end has now been changed to enable +gnu_version was not obvious. The front end has now been changed to enable the feature in clang mode for C++11 and newer modes when clang_version is at least 30400, regardless of the value of gnu_version. For example, with --c++11 --clang_version=30400: @@ -39724,7 +39724,7 @@ C++-generating back end: template arguments involving local types In C++-generating back end configurations in which CLASS_TEMPLATE_INSTANTIATIONS_IN_SOURCE_SEQUENCE_LISTS is TRUE, a use of a -template with a template type argment that is an array, pointer, or +template with a template type argument that is an array, pointer, or reference involving a local type resulted in a generated explicit specialization for that template instance in which the template argument referred to the local type from a point at which it is not in scope. This @@ -41828,7 +41828,7 @@ This is now fixed. 3/18/20 [EDGcpfe/22298] -Coroutines: Adjust behaviour of implicit try/catch block +Coroutines: Adjust behavior of implicit try/catch block C++ Committee document P1971R0 changed how a coroutine's implicit try/catch block is handled. The initial suspend point has been moved inside the try @@ -48100,7 +48100,7 @@ GNU compatibility: Default constructors for classes with const members GCC versions earlier than 4.9.0 did not implement the requirement that constructors for classes with uninitialized const members be defined as -deleted (see EDGcpfe/14040). GCC versions 4.9.0 and newer do fulfil this +deleted (see EDGcpfe/14040). GCC versions 4.9.0 and newer do fulfill this requirement. Clang always fulfilled this requirement. For example: struct A { @@ -48745,22 +48745,22 @@ expression occurred in a default template argument. Now fixed. 5/16/19 [EDGcpfe/21259] -Use Berkley SoftFloat library for floating-point arithmetic +Use Berkeley SoftFloat library for floating-point arithmetic Previously, the front end could only support floating-point types that were also supported by the host compiler (because the front end needs to be able to fold floating-point operations at compile time). A change has been made -to optionally use the Berkley SoftFloat library to perform all floating-point +to optionally use the Berkeley SoftFloat library to perform all floating-point arithmetic. This option can be selected by setting the new USE_SOFTFLOAT configuration macro to TRUE (the default is FALSE). In such configurations, the build process must be suitably modified to include the SoftFloat headers and library. -The front end was tested with Release 3e of the Berkley SoftFloat distribution. -See http://www.jhauser.us/arithmetic/SoftFloat.html for documentation and -licensing information. Performing floating-point operations in software is -slower than performing them in hardware. See the SoftFloat documentation for -details. +The front end was tested with Release 3e of the Berkeley SoftFloat +distribution. See http://www.jhauser.us/arithmetic/SoftFloat.html for +documentation and licensing information. Performing floating-point operations +in software is slower than performing them in hardware. See the SoftFloat +documentation for details. In configurations where USE_SOFTFLOAT is TRUE, USE_HOST_FP_CONVERSION_ROUTINES must be set to FALSE (see the Changes entry for EDGcpfe/21236). This provides @@ -124094,7 +124094,7 @@ name of a member function. Now fixed (an error is issued). Abort on decltype of nontype template parameter The front end aborted in decltype_from_operand in expr.c on a use of -decltype applied to a nontype template parameter identitier. Now fixed. +decltype applied to a nontype template parameter identifier. Now fixed. template inline void f() { auto x2 = sizeof (decltype(I)); // Formerly caused an abort @@ -136132,7 +136132,7 @@ numbered N1791). Various GNU C features are lowered using C99 lowering. It was previously possible to configure the front end with DO_IL_LOWERING and GNU_EXTENSIONS_ALLOWED set to TRUE and DO_C99_IL_LOWERING set to FALSE. -Ths combination left some IL constructs unlowered, which could result in +This combination left some IL constructs unlowered, which could result in aborts in the C-generating back end. Attempting to use this configuration now results in a build-time error. (As part of this change, consistency checking between DO_C99_IL_LOWERING and other configuration settings has @@ -140042,7 +140042,7 @@ as a pattern satisfying the operator. 8/23/05 Microsoft compatibility: dropping cv-qualifiers on "?" operator -Microsoft mode nows allows dropping cv-qualifiers (e.g., const) when +Microsoft mode now allows dropping cv-qualifiers (e.g., const) when converting the second or third operand of a "?" operator to the other operand type, if the conversion is from derived to base. @@ -140592,7 +140592,7 @@ with external linkage involving class types or enumeration types without linkage. Now such declarations are always diagnosed (an error in strict mode; otherwise, a warning for functions or a remark for variables). For example: - enum { no, yes } anwer; // Previously silently accepted; now a diagnostic. + enum { no, yes } answer; // Previously silently accepted; now a diagnostic. struct S { typedef struct {} *P; static P x; // Previously silently accepted; now a diagnostic. @@ -143130,7 +143130,7 @@ old-style cast, i.e., Inner2(const Inner1& r = ((Inner1)(s))); thus triggering the MSVC++ 6.0 bug. Now, when msvc_is_generated_code_target -is TRUE and msvc_target_version_number is 1200, the default argment will be +is TRUE and msvc_target_version_number is 1200, the default argument will be generated as a functional cast with no enclosing parentheses: Inner2(const Inner1& t = Inner1(s)); @@ -143322,7 +143322,7 @@ therefore issues no error on such cases. A y; // I is complete when A instantiation is done 2/8/05 Spurious error on template static data member with dependent - parenthesised initializer + parenthesized initializer A spurious error was issued if a template static data member was initialized with a parenthesized initializer of the form "T::". The error would @@ -144143,7 +144143,7 @@ currently-standard behavior for GNU C++ mode and MSVC++ 7.0. 12/21/04 Microsoft compatibility: mixed void/non-void operands to "?" operator -Microsoft C++ mode nows allows mixed void/non-void operands to the "?" +Microsoft C++ mode now allows mixed void/non-void operands to the "?" operator, with a result type of void. A previous emulation of this feature (see Changes entry of 7/1/96) was found to match the behavior of MSVC++ only in C mode and only up to version 6.0, and therefore @@ -165294,7 +165294,7 @@ an lvalue. It is now an rvalue. 9/1/00 Abort on array rvalue selected from base class of class rvalue -The front end no longers aborts on passing an array rvalue as +The front end no longer aborts on passing an array rvalue as an argument, when the array rvalue is produced by creating a class rvalue and selecting an array member of a base class of that value. @@ -169037,7 +169037,7 @@ line). 11/2/99 Pointer conversions not allowed on nontype template argument -Inplicit null pointer conversions, related-class conversions, and +Implicit null pointer conversions, related-class conversions, and conversions to "void *" are no longer allowed on a nontype template argument. See [temp.arg.nontype] paragraph 5 in the C++ standard. @@ -191946,7 +191946,7 @@ which have been corrected: address into the slot of the object address table indexed by the region table number (rather than the index number for the object address table). By coincidence, the runtime was looking for the conditional flag address - in the same wrong place, so the bugs often cancelled out, particularly + in the same wrong place, so the bugs often canceled out, particularly in small test programs. - When destroying unordered temporaries, the values placed in __eh_curr_region during the destructions were wrong and would @@ -194059,7 +194059,7 @@ could result in a compiler abort. This has been fixed. T3 t3; // Abort on this line }; -6/29/94 Overridding the severity of diagnostic messages +6/29/94 Overriding the severity of diagnostic messages Command line options have been added that allow a user to override the severity of specified diagnostics. The new severity may only be used to @@ -194070,7 +194070,7 @@ overriding mechanism. 6/29/94 Option to display error numbers -When using the severity overridding mechanism, it is sometimes helpful to +When using the severity overriding mechanism, it is sometimes helpful to know the error number of a given diagnostic. The --display_error_number command line option causes the error number to be displayed as part of the diagnostic output. @@ -200177,9 +200177,9 @@ issued in strict ANSI mode. Now a remark is issued (instead of a warning) on duplicate friend declarations. -3/23/93 IL walk routines speeded up +3/23/93 IL walk routines sped up -The routines that walk the intermediate language tree have been speeded +The routines that walk the intermediate language tree have been sped up. This was accomplished largely by moving a lot of code from il_walk.c out to a new file walk_entry.h, and then compiling it twice with different macro settings so as to avoid a lot of extra testing during diff --git a/src/decl_spec.c b/src/decl_spec.c index 7021497aef..43b0e74d85 100644 --- a/src/decl_spec.c +++ b/src/decl_spec.c @@ -152,7 +152,7 @@ string that is returned. /* Error. */ syntax_error(ec_bad_uuid_string); } else if (is_error_constant(&const_for_curr_token)) { - /* We encountered a misformed string literal. An error should + /* We encountered a malformed string literal. An error should have been issued already. */ check_assertion(is_at_least_one_error()); } else { diff --git a/src/decls.c b/src/decls.c index 6916c90263..3394db4688 100644 --- a/src/decls.c +++ b/src/decls.c @@ -11474,7 +11474,7 @@ definition of a member function of a class template. !locator->is_error) { /* Normally, we let add_to_routines_list determine which scope to add the routine to, but for proxy members nominated in friends, that - would yield a nonexisting scope; instead we just put those on the + would yield a nonexistent scope; instead we just put those on the file scope list. Similarly for friend function templates encountered in Microsoft nonreal class template instantiations. */ a_scope_depth scope_depth; diff --git a/src/expr.c b/src/expr.c index 9ea6f97565..558d4c2f76 100644 --- a/src/expr.c +++ b/src/expr.c @@ -24316,7 +24316,7 @@ static void deduce_new_array_size(a_new_parse_state *nps, a_decl_parse_state *dps) /* C++20 allows for the deduction of a new array size from the provided braced -initializer to match direct-initialization behaviour. For example: +initializer to match direct-initialization behavior. For example: int foo[]{1,2,3} new int[]{1,2,3} should both produce a 3-element array of integers, initialized to 1, 2 and 3. diff --git a/src/exprutil.c b/src/exprutil.c index f27d414f79..274dd574e4 100644 --- a/src/exprutil.c +++ b/src/exprutil.c @@ -25695,7 +25695,7 @@ conjunction of two constraints X and Y, if it is at least as constrained as each one separately (which matches intuition). The second rule is similarly intuitive: For a disjunction of two constraints X and Y to be at least as constrained as a constraint Z, each alternative (X and Y) has to be as -constrainted as Z. +constrained as Z. Consider constraints E1 and E2 expressed through conjunctions (AND) and disjunctions (OR) of atomic constraints. Let E1[k] be the set of all diff --git a/src/lang_feat.h b/src/lang_feat.h index cde404bf29..de19808360 100644 --- a/src/lang_feat.h +++ b/src/lang_feat.h @@ -424,7 +424,7 @@ static functions. C++03 did not allow that, but Core Issue 561 changed that for C++11. This is the initial value of the global variable dependent_lookup_finds_static_functions, though some modes override that value. This default macro is mostly useful as the setting for this -feature in default C++03 mode, and our judgement is that most people +feature in default C++03 mode, and our judgment is that most people would prefer to have static functions be found in that mode (other compilers, like g++ and Microsoft, have always found static functions). */ diff --git a/src/ms_attrib.c b/src/ms_attrib.c index c5954ed64f..6c04657efe 100644 --- a/src/ms_attrib.c +++ b/src/ms_attrib.c @@ -1659,7 +1659,7 @@ is set to TRUE. Note that "err" is not TRUE for an unexpected token kind. len = (a_targ_size_t)strlen(src); } else if (curr_token == tok_string_literal) { if (is_error_constant(&const_for_curr_token)) { - /* We encountered a misformed string literal. An error should + /* We encountered a malformed string literal. An error should have been issued already. */ check_assertion(is_at_least_one_error()); *err = TRUE; @@ -1728,7 +1728,7 @@ TRUE. Note that "err" is not TRUE for an unexpected token kind. *err = FALSE; if (curr_token == tok_string_literal) { if (is_error_constant(&const_for_curr_token)) { - /* We encountered a misformed string literal. An error should + /* We encountered a malformed string literal. An error should have been issued already. */ check_assertion(is_at_least_one_error()); *err = TRUE; diff --git a/util/Changes b/util/Changes index 2b5d271878..e425072ac9 100644 --- a/util/Changes +++ b/util/Changes @@ -1729,7 +1729,7 @@ when IA64_ABI is set to TRUE. 7/9/02 eccp: eliminated use of "match" option eccp made use of the "match" option of the expr command. This option is -not supported on some versions of expr (notably newer verions of Solaris). +not supported on some versions of expr (notably newer versions of Solaris). The ":" form of the option, which is more widely available, is now used instead. ------------------------------------------------------------------------------- From 4ffab1cf9a3c36aec105fe11974fef1360ed02c9 Mon Sep 17 00:00:00 2001 From: Wyatt Childers Date: Thu, 5 Nov 2026 23:12:21 +0000 Subject: [PATCH 2/3] Provide a better error message if the required dev-tool cannot be found [] --- .git-hooks/pre-push.py | 45 +++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/.git-hooks/pre-push.py b/.git-hooks/pre-push.py index 8a4dd4c571..5fea1a212e 100755 --- a/.git-hooks/pre-push.py +++ b/.git-hooks/pre-push.py @@ -105,8 +105,33 @@ def _is_excluded_file_path(path_str: str) -> bool: path_str.startswith(prefix) for prefix in EXCLUDES_PATH_PREFIXES ) +def pause_delegate_window() -> None: + '''When running as a relaunch delegate, wait for the user before the + standalone console window closes so that failure output can be read.''' + if os.environ.get('EDG_PREPUSH_DELEGATE') != '1': + return + + try: + input('Push checks failed; press Enter to close this window. ') + except EOFError: + pass + TOOL_PATH_GIT = shutil.which('git') +def find_dev_tool(name): + '''Return the path of the dev tool called name on PATH, or exit with an + error if the environment lacks it (e.g., direnv isn't applied).''' + tool_path = shutil.which(name) + if tool_path is None: + print( + f"error: the environment is missing the required tool '{name}'; " + 'put dev_tools/bin on PATH (e.g., via direnv) and push again.', + file = sys.stderr + ) + pause_delegate_window() + sys.exit(1) + return tool_path + def run_git(args, *, check = True, text = True): '''Run a git command and return the completed process.''' return subprocess.run( @@ -292,7 +317,7 @@ def check_changed_files(self, first_new_commit): if use_docker: file_change_check_proc = subprocess.run( [ - shutil.which('edg-exec'), + find_dev_tool('edg-exec'), '--', 'edg-check-file-changes', str(input_file.relative_to(mono_repo_dir).as_posix()), @@ -304,7 +329,7 @@ def check_changed_files(self, first_new_commit): else: file_change_check_proc = subprocess.run( [ - shutil.which('edg-check-file-changes'), + find_dev_tool('edg-check-file-changes'), str(input_file), str(output_file) ], @@ -418,16 +443,12 @@ def run(self) -> int: return 0 if (self.errors == 0 and not self.abort) else 1 -def pause_delegate_window() -> None: - '''When running as a relaunch delegate, wait for the user before the - standalone console window closes so that failure output can be read.''' - if os.environ.get('EDG_PREPUSH_DELEGATE') != '1': - return - - try: - input('Push checks failed; press Enter to close this window. ') - except EOFError: - pass +def wait_for_minimum_run_time() -> None: + '''Sleep until the hook has been running for MIN_RUN_SECONDS.''' + remaining = MIN_RUN_SECONDS - (time.monotonic() - START_TIME) + if remaining > 0: + print(f"Pushing in {remaining:.0f}s (Ctrl-C to cancel)...") + time.sleep(remaining) def main(): checker = PrePushChecker() From 0bdaddf66667f9a935c5768c3474783daf2f2cbf Mon Sep 17 00:00:00 2001 From: Wyatt Childers Date: Fri, 6 Nov 2026 16:41:34 +0000 Subject: [PATCH 3/3] Add a notice of where the push is going to the hook and make the script take at least 3 seconds to run [] --- .git-hooks/pre-push.py | 78 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/.git-hooks/pre-push.py b/.git-hooks/pre-push.py index 5fea1a212e..fb4e42c581 100755 --- a/.git-hooks/pre-push.py +++ b/.git-hooks/pre-push.py @@ -40,6 +40,7 @@ import subprocess import sys import tempfile +import time from pathlib import Path # Git invokes this hook with the ambient environment, which does not @@ -53,6 +54,33 @@ import edgshell import edgutil +START_TIME = time.monotonic() + +# The push isn't allowed to proceed until the hook has run for this long, so +# that there's time to read the target of the push and cancel it (e.g., with +# Ctrl-C) before it happens. +MIN_RUN_SECONDS = 3.0 + +def read_push_refs(): + '''Return the lines git supplied on standard input describing what is to be + pushed. A relaunched delegate (see below) can't read them from standard + input, so its parent passes them in the environment.''' + if os.environ.get('EDG_PREPUSH_DELEGATE') == '1': + refs_text = os.environ.get('EDG_PREPUSH_REFS', '') + elif sys.stdin.isatty(): + # Run by hand rather than by git. + refs_text = '' + else: + refs_text = sys.stdin.read() + return refs_text.splitlines() + +# Git supplies the remote name and location as arguments (both are the +# location if the push doesn't use a named remote). Read the refs before +# standard input is replaced below. +PUSH_REMOTE_NAME = sys.argv[1] if len(sys.argv) > 1 else None +PUSH_REMOTE_LOCATION = sys.argv[2] if len(sys.argv) > 2 else None +PUSH_REFS = read_push_refs() + if platform.system() == 'Windows': # The standard input of this hook carries the push ref list supplied by # git, so it cannot be used to prompt the user. When a console is @@ -68,10 +96,15 @@ delegate_proc = subprocess.run( [ sys.executable, - __file__ + __file__, + *sys.argv[1:] ], creationflags = subprocess.CREATE_NEW_CONSOLE, - env = {**os.environ, 'EDG_PREPUSH_DELEGATE': '1'} + env = { + **os.environ, + 'EDG_PREPUSH_DELEGATE': '1', + 'EDG_PREPUSH_REFS': '\n'.join(PUSH_REFS) + } ) sys.exit(delegate_proc.returncode) else: @@ -162,6 +195,37 @@ def git_show_blob(revision_path: str) -> bytes: return b'' return completed_process.stdout +ZERO_OBJECT_NAME_CHARS = set('0') + +def describe_push_ref(line): + '''Return a description of one of the ref lines git supplies to the hook + ( ), or None if line + isn't of that form.''' + fields = line.split() + if len(fields) != 4: + return None + local_ref, local_name, remote_ref, remote_name = fields + if set(local_name) <= ZERO_OBJECT_NAME_CHARS: + return f"delete {remote_ref}" + description = f"{local_ref} -> {remote_ref}" + if set(remote_name) <= ZERO_OBJECT_NAME_CHARS: + description += ' (new)' + return description + +def print_push_target(): + '''Print where the push is targeted.''' + if PUSH_REMOTE_NAME is None: + return + target = PUSH_REMOTE_NAME + if PUSH_REMOTE_LOCATION not in (None, PUSH_REMOTE_NAME): + target += f" ({PUSH_REMOTE_LOCATION})" + print(f"Pushing to {target}:\n") + for line in PUSH_REFS: + description = describe_push_ref(line) + if description is not None: + print(f" {description}") + print('') + class PrePushChecker: '''Runs EDG pre-push policy checks against new commits and changed files.''' @@ -435,6 +499,7 @@ def check_changed_files(self, first_new_commit): def run(self) -> int: '''Execute the full pre-push check sequence. Return the exit status.''' + print_push_target() first_new_commit = self.check_commits() # Skip per file checks if we didn't actually detect a new commit. @@ -450,11 +515,20 @@ def wait_for_minimum_run_time() -> None: print(f"Pushing in {remaining:.0f}s (Ctrl-C to cancel)...") time.sleep(remaining) +def wait_for_minimum_run_time() -> None: + '''Sleep until the hook has been running for MIN_RUN_SECONDS.''' + remaining = MIN_RUN_SECONDS - (time.monotonic() - START_TIME) + if remaining > 0: + print(f"Pushing in {remaining:.0f}s (Ctrl-C to cancel)...") + time.sleep(remaining) + def main(): checker = PrePushChecker() returncode = checker.run() if returncode != 0: pause_delegate_window() + else: + wait_for_minimum_run_time() sys.exit(returncode) main()