From fe952126fde8f04e6bb2d4423159f62d0bdb2ca9 Mon Sep 17 00:00:00 2001 From: Ash Berlin-Taylor Date: Sat, 10 Oct 2026 22:12:51 +0100 Subject: [PATCH 1/3] Halve the time to build docs locally by inlining spell checking Previously to see the same error in docs build as you would get in CI, you would have to run both the docs build and the spellchecking build. The issue was that the spellcheck (using sphinx-contrib's spelling) was defined as a new builder, meaning you could either build HTML, or you can check spelling. On CI, this didn't result in any extra wall-clock time as the jobs were parallelized (but this change does remove a job from the overall matrix), but locally that was almost always not the case, so this would mean full build to view the HTML, and spell checking build to look for errors, resulting in it taking twice as long. The upstream spellchecker was also slower than it should be mainly because its importable-module filter calls find_spec on every distinct identifier-like word before consulting the dictionary. Here it runs only for words that are still misspelled. Word lists, filters and `:spelling:ignore:` behave as before. This also handles errors a lot more nicely (hence all the code deletions): - They are written natively to a JSON file, not scraped from a stream - They also show up as GH annotations (Where we can attribute it to a real file) to make fixing them easier on PRs. Please note: this is lightly breaking for Airflow contributors as `breeze build-docs --docs-only` and `--spellcheck-only` are gone -- every build checks spelling now as it is essentially free (on the order of milliseconds per module). --- .github/workflows/ci-image-checks.yml | 27 +- .github/workflows/publish-docs-to-s3.yml | 9 +- .pre-commit-config.yaml | 2 + dev/breeze/doc/03_developer_tasks.rst | 14 +- dev/breeze/doc/images/output_build-docs.svg | 100 +++---- dev/breeze/doc/images/output_build-docs.txt | 2 +- .../commands/developer_commands.py | 10 - .../commands/developer_commands_config.py | 4 +- .../airflow_breeze/params/doc_build_params.py | 6 - dev/breeze/tests/test_developer_commands.py | 6 +- dev/breeze/tests/test_reproduce_ci.py | 4 +- dev/mypy/docs/conf.py | 2 +- devel-common/src/docs/README.rst | 20 +- devel-common/src/docs/build_docs.py | 273 +++--------------- devel-common/src/docs/utils/conf_constants.py | 2 +- .../src/sphinx_exts/airflow_intersphinx.py | 50 +--- .../src/sphinx_exts/airflow_spelling.py | 217 ++++++++++++++ .../sphinx_exts/docs_build/docs_builder.py | 114 ++------ .../sphinx_exts/docs_build/spelling_checks.py | 115 ++++---- .../docs_build/test_docs_builder.py | 52 ++++ .../docs_build/test_spelling_checks.py | 134 +++++++++ .../sphinx_exts/test_airflow_intersphinx.py | 51 ---- .../unit/sphinx_exts/test_airflow_spelling.py | 213 ++++++++++++++ docs/README.md | 4 +- task-sdk/docs/conf.py | 2 +- 25 files changed, 831 insertions(+), 602 deletions(-) create mode 100644 devel-common/src/sphinx_exts/airflow_spelling.py create mode 100644 devel-common/tests/unit/sphinx_exts/docs_build/test_spelling_checks.py create mode 100644 devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py diff --git a/.github/workflows/ci-image-checks.yml b/.github/workflows/ci-image-checks.yml index db0c730ebd622..7de377504880c 100644 --- a/.github/workflows/ci-image-checks.yml +++ b/.github/workflows/ci-image-checks.yml @@ -177,10 +177,6 @@ jobs: name: "Build documentation" runs-on: ${{ fromJSON(inputs.runners) }} if: inputs.docs-build == 'true' - strategy: - fail-fast: false - matrix: - flag: ["--docs-only", "--spellcheck-only"] env: GITHUB_REPOSITORY: ${{ github.repository }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -211,11 +207,11 @@ jobs: path: ./generated/_inventory_cache/ key: cache-docs-inventory-v1 id: restore-docs-inventory-cache - - name: "Building docs with ${{ matrix.flag }} flag" + - name: "Building docs" env: DOCS_LIST_AS_STRING: ${{ inputs.docs-list-as-string }} run: > - breeze build-docs ${DOCS_LIST_AS_STRING} ${{ matrix.flag }} --refresh-airflow-inventories + breeze build-docs ${DOCS_LIST_AS_STRING} --refresh-airflow-inventories - name: "Check for missing third-party inventories" id: check-missing-inventories if: always() @@ -236,12 +232,11 @@ jobs: id: get-job-url if: >- always() && - inputs.canary-run == 'true' && - matrix.flag == '--docs-only' + inputs.canary-run == 'true' shell: bash run: | JOB_URL=$(gh api "repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/jobs" \ - --jq '[.jobs[] | select(.name | test("Build documentation.*docs-only"))][0].html_url // empty') + --jq '[.jobs[] | select(.name | test("Build documentation"))][0].html_url // empty') if [[ -z "${JOB_URL}" ]]; then JOB_URL="https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" fi @@ -252,8 +247,7 @@ jobs: id: inventory-notification if: >- always() && - inputs.canary-run == 'true' && - matrix.flag == '--docs-only' + inputs.canary-run == 'true' shell: bash run: python3 scripts/ci/slack_notification_state.py env: @@ -264,8 +258,7 @@ jobs: - name: "Upload inventory notification state" if: >- always() && - inputs.canary-run == 'true' && - matrix.flag == '--docs-only' + inputs.canary-run == 'true' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: # yamllint disable-line rule:line-length @@ -276,7 +269,6 @@ jobs: - name: "Notify Slack about missing inventories (new/changed)" if: >- inputs.canary-run == 'true' && - matrix.flag == '--docs-only' && steps.inventory-notification.outputs.action == 'notify_new' uses: slackapi/slack-github-action@dcb1066f776dd043e64d0e8ba94ca15cc7e1875d # v4.0.0 with: @@ -292,7 +284,6 @@ jobs: - name: "Notify Slack about missing inventories (still not fixed)" if: >- inputs.canary-run == 'true' && - matrix.flag == '--docs-only' && steps.inventory-notification.outputs.action == 'notify_reminder' uses: slackapi/slack-github-action@dcb1066f776dd043e64d0e8ba94ca15cc7e1875d # v4.0.0 with: @@ -308,7 +299,6 @@ jobs: - name: "Notify Slack about inventory recovery" if: >- inputs.canary-run == 'true' && - matrix.flag == '--docs-only' && steps.inventory-notification.outputs.action == 'notify_recovery' uses: slackapi/slack-github-action@dcb1066f776dd043e64d0e8ba94ca15cc7e1875d # v4.0.0 with: @@ -328,9 +318,7 @@ jobs: key: cache-docs-inventory-v1 if-no-files-found: 'error' retention-days: '2' - # If we upload from multiple matrix jobs we could end up with a race condition. so just pick one job - # to be responsible for updating it. https://github.com/actions/upload-artifact/issues/506 - if: steps.restore-docs-inventory-cache.outputs.stash-hit != 'true' && matrix.flag == '--docs-only' + if: steps.restore-docs-inventory-cache.outputs.stash-hit != 'true' - name: "Upload build docs" uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: @@ -338,7 +326,6 @@ jobs: path: './generated/_build' retention-days: '7' if-no-files-found: 'error' - if: matrix.flag == '--docs-only' publish-docs: timeout-minutes: 150 diff --git a/.github/workflows/publish-docs-to-s3.yml b/.github/workflows/publish-docs-to-s3.yml index 584acbace06f5..3d57c2074f920 100644 --- a/.github/workflows/publish-docs-to-s3.yml +++ b/.github/workflows/publish-docs-to-s3.yml @@ -352,7 +352,7 @@ jobs: path: ./generated/_inventory_cache/ key: cache-docs-inventory-v1 id: restore-docs-inventory-cache - - name: "Building docs with --docs-only flag using ${{ inputs.ref }} reference breeze" + - name: "Building docs using ${{ inputs.ref }} reference breeze" id: build-sphinx-docs env: INCLUDE_DOCS: ${{ needs.build-info.outputs.include-docs }} @@ -375,7 +375,12 @@ jobs: > generated/_build/docs/sphinx-build-skipped.txt echo "sphinx-build-skipped=true" >> "${GITHUB_OUTPUT}" else - breeze build-docs ${SPHINX_INCLUDE_DOCS} --docs-only ${FAIL_ON_INVENTORIES} + # Refs whose breeze still has --docs-only check spelling in a second full build; skip it. + DOCS_ONLY="" + if grep -q '"--docs-only"' dev/breeze/src/airflow_breeze/commands/developer_commands.py; then + DOCS_ONLY="--docs-only" + fi + breeze build-docs ${SPHINX_INCLUDE_DOCS} ${DOCS_ONLY} ${FAIL_ON_INVENTORIES} fi - name: "Save docs inventory cache" uses: apache/infrastructure-actions/stash/save@61dcea11f19e2bbe1263f14d72235e8da17d3ad0 # save/v1.0.0 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3bbe42de8506f..be70256822526 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -838,6 +838,8 @@ repos: ^airflow-core/src/airflow/ui/openapi-gen/| ^airflow-core/src/airflow/api_fastapi/auth/managers/simple/ui/openapi-gen/| ^providers/edge3/src/airflow/providers/edge3/plugins/www/openapi-gen/| + ^devel-common/src/sphinx_exts/airflow_spelling\.py$| + ^devel-common/tests/unit/sphinx_exts/test_airflow_spelling\.py$| .*/dist/.*| \.go$| /go\.(mod|sum)$ diff --git a/dev/breeze/doc/03_developer_tasks.rst b/dev/breeze/doc/03_developer_tasks.rst index fe0f9f0c48ccd..02a72f69f3ec6 100644 --- a/dev/breeze/doc/03_developer_tasks.rst +++ b/dev/breeze/doc/03_developer_tasks.rst @@ -251,18 +251,8 @@ To build documentation in Breeze, use the ``build-docs`` command: Results of the build can be found in the ``generated/_build`` folder. -The documentation build consists of three steps: - -* verifying consistency of indexes -* building documentation -* spell checking - -You can choose only one stage of the two by providing ``--spellcheck-only`` or ``--docs-only`` after -extra ``--`` flag. - -.. code-block:: bash - - breeze build-docs --spellcheck-only +Spelling is checked while the documentation is built, so a single build reports both build and +spelling errors. This process can take some time, so in order to make it shorter you can filter by package, using package short ``provider id`` (might be multiple of them). diff --git a/dev/breeze/doc/images/output_build-docs.svg b/dev/breeze/doc/images/output_build-docs.svg index d9d45a1bff0b9..45d78b15e3567 100644 --- a/dev/breeze/doc/images/output_build-docs.svg +++ b/dev/breeze/doc/images/output_build-docs.svg @@ -1,4 +1,4 @@ - + - + @@ -222,15 +222,9 @@ - - - - - - - Command: build-docs + Command: build-docs @@ -259,49 +253,47 @@ Build documents. -╭─ Build scope (default is to build Python docs and spellcheck) ───────────────────────────────────────────────────────╮ -│--docs-only      -dOnly build documentation.│ -│--sdk-docs-only  Only build SDK docs. Requires at least one --sdk value to be useful.│ -│--spellcheck-only-sOnly run spell checking.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Type of build ──────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--one-pass-onlyBuilds documentation in one pass only. This is useful for debugging sphinx errors.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Cleaning inventories ───────────────────────────────────────────────────────────────────────────────────────────────╮ -│--clean-build                            Cleans the build directory before building the documentation. Does not   │ -│delete inventory cache (use --clean-inventory-cache for that).           │ -│--clean-inventory-cache                  Cleans the inventory cache before fetching inventories.│ -│--refresh-airflow-inventories            When set, only airflow package inventories will be refreshed, regardless │ -│if they are already downloaded. With `--clean-build` - everything is     │ -│cleaned.                                                                 │ -│--fail-on-missing-third-party-inventoriesFail the build if any third-party inventory cannot be downloaded. By     │ -│default, missing third-party inventories are warned about but do not fail│ -│the build.                                                               │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Filtering options ──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--sdk                        Select SDK toolchain(s). (java | typescript)│ -│--package-filter             Filter(s) to use more than one can be specified. You can use glob pattern matching   │ -│the full package name, for example `apache-airflow-providers-*`. Useful when you want│ -│to selectseveral similarly named packages together. (TEXT)│ -│--include-not-ready-providersWhether to include providers that are not yet ready to be released.│ -│--include-removed-providers  Whether to include providers that are removed.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Misc options ───────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--include-commits   Include commits in the documentation.│ -│--github-repository -gGitHub repository used to pull, push run images. [default: apache/airflow](TEXT)│ -│--builder           Buildx builder used to perform `docker buildx build` commands. [default: autodetect]│ -│(TEXT)│ -│--distributions-listOptional, contains space separated list of package ids that are processed for             │ -│documentation building, and document publishing. It is an easier alternative to adding    │ -│individual packages as arguments to every command. This overrides the packages passed as  │ -│arguments. (TEXT)│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Common options ─────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ -│--verbose-vPrint verbose information about performed steps.│ -│--answer -aForce answer to questions. (y | n | q | yes | no | quit)│ -│--help   -hShow this message and exit.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Build scope (default is to build Python docs and check their spelling) ─────────────────────────────────────────────╮ +│--sdk-docs-onlyOnly build SDK docs. Requires at least one --sdk value to be useful.│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Type of build ──────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--one-pass-onlyBuilds documentation in one pass only. This is useful for debugging sphinx errors.│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Cleaning inventories ───────────────────────────────────────────────────────────────────────────────────────────────╮ +│--clean-build                            Cleans the build directory before building the documentation. Does not   │ +│delete inventory cache (use --clean-inventory-cache for that).           │ +│--clean-inventory-cache                  Cleans the inventory cache before fetching inventories.│ +│--refresh-airflow-inventories            When set, only airflow package inventories will be refreshed, regardless │ +│if they are already downloaded. With `--clean-build` - everything is     │ +│cleaned.                                                                 │ +│--fail-on-missing-third-party-inventoriesFail the build if any third-party inventory cannot be downloaded. By     │ +│default, missing third-party inventories are warned about but do not fail│ +│the build.                                                               │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Filtering options ──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--sdk                        Select SDK toolchain(s). (java | typescript)│ +│--package-filter             Filter(s) to use more than one can be specified. You can use glob pattern matching   │ +│the full package name, for example `apache-airflow-providers-*`. Useful when you want│ +│to selectseveral similarly named packages together. (TEXT)│ +│--include-not-ready-providersWhether to include providers that are not yet ready to be released.│ +│--include-removed-providers  Whether to include providers that are removed.│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Misc options ───────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--include-commits   Include commits in the documentation.│ +│--github-repository -gGitHub repository used to pull, push run images. [default: apache/airflow](TEXT)│ +│--builder           Buildx builder used to perform `docker buildx build` commands. [default: autodetect]│ +│(TEXT)│ +│--distributions-listOptional, contains space separated list of package ids that are processed for             │ +│documentation building, and document publishing. It is an easier alternative to adding    │ +│individual packages as arguments to every command. This overrides the packages passed as  │ +│arguments. (TEXT)│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Common options ─────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ +│--verbose-vPrint verbose information about performed steps.│ +│--answer -aForce answer to questions. (y | n | q | yes | no | quit)│ +│--help   -hShow this message and exit.│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ diff --git a/dev/breeze/doc/images/output_build-docs.txt b/dev/breeze/doc/images/output_build-docs.txt index a446d44aa41a0..5105c9d092ba4 100644 --- a/dev/breeze/doc/images/output_build-docs.txt +++ b/dev/breeze/doc/images/output_build-docs.txt @@ -1 +1 @@ -c4760da49161465146c45c7c70f45a41 +585c88f391fe92933f3d30f4bff0be5b diff --git a/dev/breeze/src/airflow_breeze/commands/developer_commands.py b/dev/breeze/src/airflow_breeze/commands/developer_commands.py index d6250fba9a070..fdee4b631042f 100644 --- a/dev/breeze/src/airflow_breeze/commands/developer_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/developer_commands.py @@ -892,7 +892,6 @@ def _build_python_docs( clean_inventory_cache: bool, refresh_airflow_inventories: bool, fail_on_missing_third_party_inventories: bool, - docs_only: bool, github_repository: str, include_not_ready_providers: bool, include_removed_providers: bool, @@ -900,7 +899,6 @@ def _build_python_docs( one_pass_only: bool, package_filter: tuple[str, ...], distributions_list: str, - spellcheck_only: bool, doc_packages: tuple[str, ...], ): # Docs are always built on the default Python. The Sphinx configuration mocks third-party @@ -948,8 +946,6 @@ def _build_python_docs( doc_packages = docs_list_as_tuple or doc_packages doc_builder = DocBuildParams( package_filter=package_filter, - docs_only=docs_only, - spellcheck_only=spellcheck_only, one_pass_only=one_pass_only, include_commits=include_commits, fail_on_missing_third_party_inventories=fail_on_missing_third_party_inventories, @@ -1004,7 +1000,6 @@ def _build_python_docs( help="Fail the build if any third-party inventory cannot be downloaded. " "By default, missing third-party inventories are warned about but do not fail the build.", ) -@click.option("-d", "--docs-only", help="Only build documentation.", is_flag=True) @click.option( "--include-commits", help="Include commits in the documentation.", is_flag=True, envvar="INCLUDE_COMMITS" ) @@ -1033,7 +1028,6 @@ def _build_python_docs( "building, and document publishing. It is an easier alternative to adding individual packages as" " arguments to every command. This overrides the packages passed as arguments.", ) -@click.option("-s", "--spellcheck-only", help="Only run spell checking.", is_flag=True) @option_sdk @click.option( "--sdk-docs-only", @@ -1049,7 +1043,6 @@ def build_docs( clean_inventory_cache: bool, refresh_airflow_inventories: bool, fail_on_missing_third_party_inventories: bool, - docs_only: bool, github_repository: str, include_not_ready_providers: bool, include_removed_providers: bool, @@ -1057,7 +1050,6 @@ def build_docs( one_pass_only: bool, package_filter: tuple[str, ...], distributions_list: str, - spellcheck_only: bool, sdk: tuple[str, ...], sdk_docs_only: bool, doc_packages: tuple[str, ...], @@ -1079,7 +1071,6 @@ def build_docs( clean_inventory_cache=clean_inventory_cache, refresh_airflow_inventories=refresh_airflow_inventories, fail_on_missing_third_party_inventories=fail_on_missing_third_party_inventories, - docs_only=docs_only, github_repository=github_repository, include_not_ready_providers=include_not_ready_providers, include_removed_providers=include_removed_providers, @@ -1087,7 +1078,6 @@ def build_docs( one_pass_only=one_pass_only, package_filter=package_filter, distributions_list=distributions_list, - spellcheck_only=spellcheck_only, doc_packages=doc_packages, ) # Every requested SDK is built even if an earlier one failed, so a single run reports diff --git a/dev/breeze/src/airflow_breeze/commands/developer_commands_config.py b/dev/breeze/src/airflow_breeze/commands/developer_commands_config.py index 8d53feb5a5d21..97e85a7d3121b 100644 --- a/dev/breeze/src/airflow_breeze/commands/developer_commands_config.py +++ b/dev/breeze/src/airflow_breeze/commands/developer_commands_config.py @@ -348,8 +348,8 @@ ], "breeze build-docs": [ { - "name": "Build scope (default is to build Python docs and spellcheck)", - "options": ["--docs-only", "--sdk-docs-only", "--spellcheck-only"], + "name": "Build scope (default is to build Python docs and check their spelling)", + "options": ["--sdk-docs-only"], }, { "name": "Type of build", diff --git a/dev/breeze/src/airflow_breeze/params/doc_build_params.py b/dev/breeze/src/airflow_breeze/params/doc_build_params.py index fc5e1df967489..13e99df06b73b 100644 --- a/dev/breeze/src/airflow_breeze/params/doc_build_params.py +++ b/dev/breeze/src/airflow_breeze/params/doc_build_params.py @@ -25,8 +25,6 @@ @dataclass class DocBuildParams: package_filter: tuple[str, ...] - docs_only: bool - spellcheck_only: bool short_doc_packages: tuple[str, ...] one_pass_only: bool = False include_commits: bool = False @@ -37,10 +35,6 @@ class DocBuildParams: @property def args_doc_builder(self) -> list[str]: doc_args = [] - if self.docs_only: - doc_args.append("--docs-only") - if self.spellcheck_only: - doc_args.append("--spellcheck-only") if self.one_pass_only: doc_args.append("--one-pass-only") if self.include_commits: diff --git a/dev/breeze/tests/test_developer_commands.py b/dev/breeze/tests/test_developer_commands.py index 13968a7ce6591..6824a59a71747 100644 --- a/dev/breeze/tests/test_developer_commands.py +++ b/dev/breeze/tests/test_developer_commands.py @@ -97,15 +97,13 @@ def _invoke(self, runner: CliRunner, args: list[str], env: dict[str, str] | None def test_environment_python_does_not_change_the_docs_build(self, runner): # PYTHON_MAJOR_MINOR_VERSION is set on every job of the docs publishing workflow, so an # option reading it silently decided what the docs were built with. - mock_build, mock_shell = self._invoke( - runner, ["--docs-only"], env={"PYTHON_MAJOR_MINOR_VERSION": "3.12"} - ) + mock_build, mock_shell = self._invoke(runner, [], env={"PYTHON_MAJOR_MINOR_VERSION": "3.12"}) assert mock_build.call_args.kwargs["command_params"].python == DEFAULT_PYTHON_MAJOR_MINOR_VERSION assert mock_shell.call_args.args[0].python == DEFAULT_PYTHON_MAJOR_MINOR_VERSION def test_python_option_is_rejected(self, runner): - result = runner.invoke(build_docs, ["--python", "3.12", "--docs-only"]) + result = runner.invoke(build_docs, ["--python", "3.12"]) assert result.exit_code != 0 assert "no such option" in result.output.lower() diff --git a/dev/breeze/tests/test_reproduce_ci.py b/dev/breeze/tests/test_reproduce_ci.py index 6b4b6628bb9d3..dd6490d3ae4a4 100644 --- a/dev/breeze/tests/test_reproduce_ci.py +++ b/dev/breeze/tests/test_reproduce_ci.py @@ -125,7 +125,7 @@ def test_print_local_reproduction_renders_copyable_commands(mock_get_console, mo [ ReproductionCommand(argv=["git", "checkout", "abc123"], comment="Check out the same commit"), ReproductionCommand( - argv=["breeze", "build-docs", "--docs-only"], + argv=["breeze", "build-docs", "--one-pass-only"], comment="Run the same Breeze command locally", ), ] @@ -137,7 +137,7 @@ def test_print_local_reproduction_renders_copyable_commands(mock_get_console, mo rendered_output = mock_get_console.return_value.print.call_args_list[2].args[0] assert "# 1. Check out the same commit" in rendered_output assert "git checkout abc123" in rendered_output - assert "breeze build-docs --docs-only" in rendered_output + assert "breeze build-docs --one-pass-only" in rendered_output bottom_ruler = mock_get_console.return_value.print.call_args_list[3].args[0] assert "─" * 80 in bottom_ruler diff --git a/dev/mypy/docs/conf.py b/dev/mypy/docs/conf.py index 713238f8e932b..4a5e883f8dbfc 100644 --- a/dev/mypy/docs/conf.py +++ b/dev/mypy/docs/conf.py @@ -46,7 +46,7 @@ extensions = [ "sphinx.ext.intersphinx", "airflow_intersphinx", - "sphinxcontrib.spelling", + "airflow_spelling", ] html_theme = "sphinx_airflow_theme" diff --git a/devel-common/src/docs/README.rst b/devel-common/src/docs/README.rst index 02365353929ed..25aa5b664124e 100644 --- a/devel-common/src/docs/README.rst +++ b/devel-common/src/docs/README.rst @@ -90,12 +90,8 @@ Building documentation To generate a local version of the docs you can use `<../dev/breeze/doc/README.rst>`_. -The documentation build consists of verifying consistency of documentation and two steps: - -* spell checking -* building documentation - -You can choose to run the complete build, to build all the docs and run spellcheck. Or, you can use package names and the optional flags, ``--spellcheck-only`` or ``--docs-only`` to choose the scope of the build. +Spelling is checked while the documentation is built, so one build reports both build and spelling errors. +You can use package names to limit the build. Build all docs and spell check them: @@ -103,18 +99,6 @@ Build all docs and spell check them: breeze build-docs -Just run spellcheck: - -.. code-block:: bash - - breeze build-docs --spellcheck-only - -Build docs without checking spelling: - -.. code-block:: bash - - breeze build-docs --docs-only - Build documentation of just one provider package by calling the ``PACKAGE_ID``. .. code-block:: bash diff --git a/devel-common/src/docs/build_docs.py b/devel-common/src/docs/build_docs.py index 54ddeb3102bb9..0683e166b11bf 100755 --- a/devel-common/src/docs/build_docs.py +++ b/devel-common/src/docs/build_docs.py @@ -27,7 +27,7 @@ from collections import defaultdict from collections.abc import Callable, Iterable, Sequence from pathlib import Path -from typing import TYPE_CHECKING, Any, NamedTuple, TypeVar +from typing import TYPE_CHECKING, NamedTuple, TypeVar if TYPE_CHECKING: from click import Context, Parameter @@ -49,7 +49,11 @@ from sphinx_exts.docs_build.fetch_inventories import fetch_inventories from sphinx_exts.docs_build.github_action_utils import with_group from sphinx_exts.docs_build.package_filter import find_packages_to_build -from sphinx_exts.docs_build.spelling_checks import SpellingError, display_spelling_error_summary +from sphinx_exts.docs_build.spelling_checks import ( + SpellingError, + display_spelling_error_summary, + emit_github_annotations, +) TEXT_RED = "\033[31m" TEXT_RESET = "\033[0m" @@ -128,10 +132,6 @@ def _promote_new_flags(): console.print("In the root of Airflow repo, you can build all packages together:") console.print(" [bright_blue]uv run --group docs build-docs[/]") console.print() - console.print("You can also use other extra flags to iterate faster:") - console.print(" [bright_blue]--docs-only - Only build documentation[/]") - console.print(" [bright_blue]--spellcheck-only - Only perform spellchecking[/]") - console.print() console.print("You can list all packages you can build:") console.print() console.print(" [bright_blue]--list-packages - Shows the list of packages you can build[/]") @@ -164,15 +164,7 @@ class BuildDocsResult(NamedTuple): package_name: str log_file_name: Path errors: list[DocBuildError] - - -class SpellCheckResult(NamedTuple): - """Result of spellcheck.""" - - package_name: str - log_file_name: Path spelling_errors: list[SpellingError] - build_errors: list[DocBuildError] def perform_docs_build_for_single_package(build_specification: BuildSpecification) -> BuildDocsResult: @@ -183,39 +175,18 @@ def perform_docs_build_for_single_package(build_specification: BuildSpecificatio f"[bright_blue]{build_specification.package_name:60}:[/] Building documentation" + (" (autobuild)" if build_specification.is_autobuild else "") ) - result = BuildDocsResult( + errors, spelling_errors = builder.build_sphinx_docs(verbose=build_specification.verbose) + return BuildDocsResult( package_name=build_specification.package_name, - errors=builder.build_sphinx_docs( - verbose=build_specification.verbose, - ), log_file_name=builder.log_build_filename, - ) - return result - - -def perform_spell_check_for_single_package(build_specification: BuildSpecification) -> SpellCheckResult: - """Performs single package spell check.""" - builder = AirflowDocsBuilder(package_name=build_specification.package_name) - builder.is_autobuild = build_specification.is_autobuild - console.print(f"[bright_blue]{build_specification.package_name:60}:[/] Checking spelling started") - spelling_errors, build_errors = builder.check_spelling( - verbose=build_specification.verbose, - ) - result = SpellCheckResult( - package_name=build_specification.package_name, + errors=errors, spelling_errors=spelling_errors, - build_errors=build_errors, - log_file_name=builder.log_spelling_filename, ) - console.print(f"[bright_blue]{build_specification.package_name:60}:[/] Checking spelling completed") - return result def build_docs_for_packages( packages_to_build: list[str], is_autobuild: bool, - docs_only: bool, - spellcheck_only: bool, jobs: int, verbose: bool, ) -> tuple[dict[str, list[DocBuildError]], dict[str, list[SpellingError]]]: @@ -228,93 +199,30 @@ def build_docs_for_packages( builder = AirflowDocsBuilder(package_name=package_name) builder.is_autobuild = is_autobuild builder.clean_files() - if jobs > 1 and len(packages_to_build) > 1: - run_in_parallel( - all_build_errors=all_build_errors, - all_spelling_errors=all_spelling_errors, - packages_to_build=packages_to_build, - docs_only=docs_only, - jobs=jobs, - spellcheck_only=spellcheck_only, - verbose=verbose, - ) - else: - run_sequentially( - all_build_errors=all_build_errors, - all_spelling_errors=all_spelling_errors, - packages_to_build=packages_to_build, - is_autobuild=is_autobuild, - docs_only=docs_only, - spellcheck_only=spellcheck_only, - verbose=verbose, - ) - return all_build_errors, all_spelling_errors - - -def run_sequentially( - all_build_errors, - all_spelling_errors, - is_autobuild, - packages_to_build, - docs_only, - spellcheck_only, - verbose, -): - """Run both - spellcheck and docs build sequentially without multiprocessing""" - if not spellcheck_only: - for package_name in packages_to_build: - build_result = perform_docs_build_for_single_package( - build_specification=BuildSpecification( - package_name=package_name, - is_autobuild=is_autobuild, - verbose=verbose, - ) - ) - if build_result.errors: - all_build_errors[package_name].extend(build_result.errors) - print_build_output(build_result) - if not docs_only: - for package_name in packages_to_build: - spellcheck_result = perform_spell_check_for_single_package( - build_specification=BuildSpecification( - package_name=package_name, - is_autobuild=is_autobuild, - verbose=verbose, + build_specifications = [ + BuildSpecification(package_name=package_name, is_autobuild=is_autobuild, verbose=verbose) + for package_name in sort_heaviest_first(packages_to_build) + ] + with with_group("Running docs building"): + console.print() + if jobs > 1 and len(packages_to_build) > 1: + with multiprocessing.Pool(processes=jobs) as pool: + # chunksize=1 hands packages out one at a time, so a worker that finishes early picks up + # the next package instead of the fixed chunk pool.map would have pre-assigned to it. + results = list( + pool.imap_unordered( + perform_docs_build_for_single_package, build_specifications, chunksize=1 + ) ) - ) - if spellcheck_result.spelling_errors: - all_spelling_errors[package_name].extend(spellcheck_result.spelling_errors) - if spellcheck_only: - all_build_errors[package_name].extend(spellcheck_result.build_errors) - print_spelling_output(spellcheck_result) - - -def run_in_parallel( - all_build_errors: dict[str, list[DocBuildError]], - all_spelling_errors: dict[str, list[SpellingError]], - packages_to_build: list[str], - docs_only: bool, - jobs: int, - spellcheck_only: bool, - verbose: bool, -): - """Run both - spellcheck and docs build sequentially without multiprocessing""" - with multiprocessing.Pool(processes=jobs) as pool: - if not spellcheck_only: - run_docs_build_in_parallel( - all_build_errors=all_build_errors, - packages_to_build=packages_to_build, - verbose=verbose, - pool=pool, - ) - if not docs_only: - run_spell_check_in_parallel( - all_spelling_errors=all_spelling_errors, - all_build_errors=all_build_errors, - packages_to_build=packages_to_build, - verbose=verbose, - pool=pool, - ) + else: + results = [perform_docs_build_for_single_package(spec) for spec in build_specifications] + for result in results: + if result.errors: + all_build_errors[result.package_name].extend(result.errors) + print_build_output(result) + if result.spelling_errors: + all_spelling_errors[result.package_name].extend(result.spelling_errors) + return all_build_errors, all_spelling_errors def print_build_output(result: BuildDocsResult): @@ -356,78 +264,6 @@ def sort_heaviest_first(packages: list[str]) -> list[str]: return sorted(packages, key=lambda package_name: (-_estimated_build_weight(package_name), package_name)) -def run_docs_build_in_parallel( - all_build_errors: dict[str, list[DocBuildError]], - packages_to_build: list[str], - verbose: bool, - pool: Any, # Cannot use multiprocessing types here: https://github.com/python/typeshed/issues/4266 -): - """Runs documentation building in parallel.""" - doc_build_specifications: list[BuildSpecification] = [] - with with_group("Scheduling documentation to build"): - for package_name in sort_heaviest_first(packages_to_build): - console.print(f"[bright_blue]{package_name:60}:[/] Scheduling documentation to build") - doc_build_specifications.append( - BuildSpecification( - is_autobuild=False, - package_name=package_name, - verbose=verbose, - ) - ) - with with_group("Running docs building"): - console.print() - # chunksize=1 hands packages out one at a time, so a worker that finishes early picks up the - # next package instead of the fixed chunk pool.map would have pre-assigned to it. - result_list = list( - pool.imap_unordered(perform_docs_build_for_single_package, doc_build_specifications, chunksize=1) - ) - for result in result_list: - if result.errors: - all_build_errors[result.package_name].extend(result.errors) - print_build_output(result) - - -def print_spelling_output(result: SpellCheckResult): - """Prints output of spell check job.""" - with with_group(f"{TEXT_RED}Output for spelling check: {result.package_name}{TEXT_RESET}"): - console.print() - console.print(f"[bright_blue]{result.package_name:60}: " + "#" * 80) - with open(result.log_file_name) as output: - for line in output.read().splitlines(): - console.print(f"{result.package_name:60} {line}") - console.print(f"[bright_blue]{result.package_name:60}: " + "#" * 80) - console.print() - - -def run_spell_check_in_parallel( - all_spelling_errors: dict[str, list[SpellingError]], - all_build_errors: dict[str, list[DocBuildError]], - packages_to_build: list[str], - verbose: bool, - pool, -): - """Runs spell check in parallel.""" - spell_check_specifications: list[BuildSpecification] = [] - with with_group("Scheduling spell checking of documentation"): - for package_name in sort_heaviest_first(packages_to_build): - console.print(f"[bright_blue]{package_name:60}:[/] Scheduling spellchecking") - spell_check_specifications.append( - BuildSpecification(package_name=package_name, is_autobuild=False, verbose=verbose) - ) - with with_group("Running spell checking of documentation"): - console.print() - result_list = list( - pool.imap_unordered( - perform_spell_check_for_single_package, spell_check_specifications, chunksize=1 - ) - ) - for result in result_list: - if result.spelling_errors: - all_spelling_errors[result.package_name].extend(result.spelling_errors) - all_build_errors[result.package_name].extend(result.build_errors) - print_spelling_output(result) - - def display_packages_summary( build_errors: dict[str, list[DocBuildError]], spelling_errors: dict[str, list[SpellingError]] ): @@ -449,20 +285,16 @@ def display_packages_summary( def print_build_errors_and_exit( build_errors: dict[str, list[DocBuildError]], spelling_errors: dict[str, list[SpellingError]], - spellcheck_only: bool, ) -> None: """Prints build errors and exists.""" if build_errors or spelling_errors: if build_errors: - if spellcheck_only: - console.print("[warning]There were some build errors remaining.") - console.print() - else: - display_errors_summary(build_errors) - console.print() + display_errors_summary(build_errors) + console.print() if spelling_errors: display_spelling_error_summary(spelling_errors) console.print() + emit_github_annotations(spelling_errors) console.print("The documentation has errors.") display_packages_summary(build_errors, spelling_errors) console.print() @@ -494,10 +326,6 @@ def is_command_available(command): click.rich_click.OPTION_GROUPS = { "build-docs": [ - { - "name": "Build scope (default is to build docs and spellcheck)", - "options": ["--docs-only", "--spellcheck-only"], - }, { "name": "Type of build", "options": ["--autobuild", "--one-pass-only"], @@ -536,7 +364,7 @@ def is_command_available(command): is_flag=True, help="Starts server, serving the build docs (sphinx-autobuild) rebuilding and " "refreshing the docs when they change on the disk. " - "Implies --verbose, --docs-only and --one-pass-only", + "Implies --verbose and --one-pass-only", ) @click.option("--one-pass-only", is_flag=True, help="Do not attempt multiple builds on error") @click.option( @@ -550,8 +378,6 @@ def is_command_available(command): "only that package is selected to build. " "If the command is run in the root of the Airflow repo, all packages are selected to be built.", ) -@click.option("--docs-only", is_flag=True, help="Only build documentation") -@click.option("--spellcheck-only", is_flag=True, help="Only perform spellchecking") @click.option( "--include-commits", help="Include commits in the documentation.", envvar="INCLUDE_COMMITS", is_flag=True ) @@ -610,8 +436,6 @@ def build_docs( package_filters, clean_build, clean_inventory_cache, - docs_only, - spellcheck_only, include_commits, jobs, list_packages, @@ -673,10 +497,7 @@ def build_docs( console.print("[red]You cannot use more than 1 package with --autobuild. Quitting.[/]") sys.exit(1) if autobuild: - console.print( - "[yellow]Autobuild mode is enabled. Forcing --docs-only, --one-pass-only and --verbose[/]" - ) - docs_only = True + console.print("[yellow]Autobuild mode is enabled. Forcing --one-pass-only and --verbose[/]") verbose = True one_pass_only = True @@ -701,8 +522,6 @@ def build_docs( package_build_errors, package_spelling_errors = build_docs_for_packages( packages_to_build=priority_packages, is_autobuild=autobuild, - docs_only=docs_only, - spellcheck_only=spellcheck_only, jobs=jobs, verbose=verbose, ) @@ -718,8 +537,6 @@ def build_docs( package_build_errors, package_spelling_errors = build_docs_for_packages( packages_to_build=packages_to_build if len(priority_packages) > 1 else normal_packages, is_autobuild=autobuild, - docs_only=docs_only, - spellcheck_only=spellcheck_only, jobs=jobs, verbose=verbose, ) @@ -730,15 +547,13 @@ def build_docs( if not one_pass_only: # Packages that failed on a cross-reference to a package built later in the same pass are - # built again now that the inventory exists (spelling builds write one too, see - # airflow_intersphinx). The second retry covers a change spanning A -> B -> C dependencies. + # built again now that the inventory exists. The second retry covers a change spanning + # A -> B -> C dependencies. for _ in range(2): package_build_errors = retry_building_docs_if_needed( all_build_errors=all_build_errors, all_spelling_errors=all_spelling_errors, autobuild=autobuild, - docs_only=docs_only, - spellcheck_only=spellcheck_only, jobs=jobs, verbose=verbose, package_build_errors=package_build_errors, @@ -749,19 +564,13 @@ def build_docs( if not package_filters: _promote_new_flags() - print_build_errors_and_exit( - all_build_errors, - all_spelling_errors, - spellcheck_only, - ) + print_build_errors_and_exit(all_build_errors, all_spelling_errors) def retry_building_docs_if_needed( all_build_errors: dict[str, list[DocBuildError]], all_spelling_errors: dict[str, list[SpellingError]], autobuild: bool, - docs_only: bool, - spellcheck_only: bool, jobs: int, verbose: bool, package_build_errors: dict[str, list[DocBuildError]], @@ -784,8 +593,6 @@ def retry_building_docs_if_needed( package_build_errors, package_spelling_errors = build_docs_for_packages( packages_to_build=to_retry_packages, is_autobuild=autobuild, - docs_only=docs_only, - spellcheck_only=spellcheck_only, jobs=jobs, verbose=verbose, ) diff --git a/devel-common/src/docs/utils/conf_constants.py b/devel-common/src/docs/utils/conf_constants.py index 9ae6c0f21874e..9891805565308 100644 --- a/devel-common/src/docs/utils/conf_constants.py +++ b/devel-common/src/docs/utils/conf_constants.py @@ -89,7 +89,7 @@ def get_rst_epilogue(package_version: str, airflow_core: bool) -> str: "python_builtin_xrefs", "common_compat_alias", "sphinxcontrib.mermaid", - "sphinxcontrib.spelling", + "airflow_spelling", "sphinx_airflow_theme", "redirects", "substitution_extensions", diff --git a/devel-common/src/sphinx_exts/airflow_intersphinx.py b/devel-common/src/sphinx_exts/airflow_intersphinx.py index a5b83bfcff4d7..609b3ab85484a 100644 --- a/devel-common/src/sphinx_exts/airflow_intersphinx.py +++ b/devel-common/src/sphinx_exts/airflow_intersphinx.py @@ -22,7 +22,6 @@ from typing import TYPE_CHECKING, Any from provider_yaml_utils import load_package_data -from sphinx.util.inventory import InventoryFile if TYPE_CHECKING: from sphinx.application import Sphinx @@ -33,50 +32,12 @@ def _inventory_for(package_name: str, versioned: bool) -> Path: - """ - Prefer an inventory written earlier in this run, fall back to the one fetched from the published docs. - - The html builder writes its inventory at the root of the build directory; a spelling-only build - writes one into its own output directory (see ``_dump_inventory_after_spelling_build``, the - directory name mirrors ``AirflowDocsBuilder.log_spelling_output_dir``). - """ + """Prefer the inventory built earlier in this run, fall back to the one fetched from the published docs.""" build_dir = GENERATED_PATH / "_build" / "docs" / package_name / ("stable" if versioned else "") - built_this_run = ( - build_dir / INVENTORY_FILENAME, - build_dir / f"output-spelling-results-{package_name}" / INVENTORY_FILENAME, - ) - downloaded = GENERATED_PATH / "_inventory_cache" / package_name / INVENTORY_FILENAME - return next((inventory for inventory in built_this_run if inventory.exists()), downloaded) - - -class _HtmlTargetUris: - """Gives inventory entries the URIs the html build would, so the file is an ordinary inventory.""" - - @staticmethod - def get_target_uri(docname: str, typ: str | None = None) -> str: - return f"{docname}.html" - - -def _dump_inventory_after_spelling_build(app: Sphinx, exception: Exception | None) -> None: - """ - Write ``objects.inv`` after a successful spelling build. - - Cross-references are resolved by every builder, so a ``--spellcheck-only`` run of package B fails - on a label that package A added in the same change unless A's fresh inventory is available. Only - the html builder writes inventories, which used to force the spellcheck run to rebuild every - package a second time with html. Writing the inventory here lets a spellcheck-only run resolve - references the same way a docs build does. - - "Successful" means the build ran to completion, not that it passed: under ``-W`` Sphinx (8.1 and - 9.x) counts warnings and sets a non-zero exit status at the end instead of aborting, so a package - with a genuine misspelling or an unresolved reference still reaches this hook with - ``exception=None`` and still publishes its complete inventory for the packages that depend on it. - Only a build that crashed part-way (``exception`` set) is skipped, because its environment may - not hold every document. - """ - if exception is not None or app.builder.name != "spelling": - return - InventoryFile.dump(os.path.join(app.outdir, INVENTORY_FILENAME), app.env, _HtmlTargetUris()) # type: ignore[arg-type] + built_this_run = build_dir / INVENTORY_FILENAME + if built_this_run.exists(): + return built_this_run + return GENERATED_PATH / "_inventory_cache" / package_name / INVENTORY_FILENAME def _create_init_py(app, config): @@ -133,7 +94,6 @@ def _generate_provider_intersphinx_mapping() -> dict[str, tuple[str, tuple[str, def setup(app: Sphinx): """Sets the plugin up""" app.connect("config-inited", _create_init_py) - app.connect("build-finished", _dump_inventory_after_spelling_build) return {"version": "builtin", "parallel_read_safe": True, "parallel_write_safe": True} diff --git a/devel-common/src/sphinx_exts/airflow_spelling.py b/devel-common/src/sphinx_exts/airflow_spelling.py new file mode 100644 index 0000000000000..4a83606490450 --- /dev/null +++ b/devel-common/src/sphinx_exts/airflow_spelling.py @@ -0,0 +1,217 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Spell check every document while the html builder writes it. + +``sphinxcontrib.spelling`` ships a separate ``spelling`` builder, which means building every package +twice. This extension runs the same checks - its tokenizer, filters, dictionary, word lists and the +``spelling`` domain's directive and roles - on each resolved doctree of the html build instead, and +writes what it finds to the JSON file named by the ``airflow_spelling_output`` config value. Leaving +that value empty disables the check. +""" + +from __future__ import annotations + +import json +import os +import re +import tempfile +from collections.abc import Iterator, Sequence +from pathlib import Path +from typing import TYPE_CHECKING, Any + +import enchant +from docutils import nodes +from docutils.utils import get_source_line +from docutils.utils.smartquotes import smartchars +from enchant.tokenize import EmailFilter, WikiWordFilter, get_tokenizer +from sphinx.util.matching import Matcher +from sphinxcontrib.spelling import filters +from sphinxcontrib.spelling.builder import SpellingBuilder + +if TYPE_CHECKING: + from docutils.parsers.rst.states import Inliner + from sphinx.application import Sphinx + from sphinx.config import Config + from sphinx.environment import BuildEnvironment + +# Enchant's tokenizer starts by splitting on whitespace, so a whitespace-delimited chunk is checked the +# same way on its own as within its paragraph. +_CHUNK = re.compile(r"\S+") + + +# SmartQuotes swaps typed quotes, dashes and "..." for characters the tokenizer and filters do not understand +# ("didn’t" splits into "didn" and "t", "isinstance…" escapes PythonBuiltinsFilter), so text is checked as typed. +# The docs are English, whose quotes are single characters; some languages' are not and str.maketrans would refuse. +def _build_unsmarten_table() -> dict[int, str]: + double_open, double_close, single_open, single_close = tuple(smartchars.quotes["en"]) + return str.maketrans( + { + double_open: '"', + double_close: '"', + single_open: "'", + single_close: "'", + smartchars.apostrophe: "'", + smartchars.endash: "--", + smartchars.emdash: "---", + smartchars.ellipsis: "...", + } + ) + + +_UNSMARTEN = _build_unsmarten_table() +_RUN_ATTRIBUTE = "_airflow_spelling_run" + + +class SpellChecker: + """Finds misspelled words the way ``sphinxcontrib.spelling``'s builder does, with caching.""" + + def __init__(self, config: Config, srcdir: str | os.PathLike) -> None: + cheap_filters: list[Any] = [filters.ContractionFilter, EmailFilter] + if config.spelling_ignore_wiki_words: + cheap_filters.append(WikiWordFilter) + if config.spelling_ignore_acronyms: + cheap_filters.append(filters.AcronymFilter) + if config.spelling_ignore_python_builtins: + cheap_filters.append(filters.PythonBuiltinsFilter) + cheap_filters.extend(config.spelling_filters) + # Both only ever drop tokens and both are slow (find_spec over sys.path, git log), so they are + # applied only to chunks that are still misspelled after the cheap filters. + slow_filters: list[Any] = [] + if config.spelling_ignore_importable_modules: + slow_filters.append(filters.ImportableModuleFilter) + if config.spelling_ignore_contributor_names: + slow_filters.append(filters.ContributorFilter) + + word_lists = config.spelling_word_list_filename or ["spelling_wordlist.txt"] + if isinstance(word_lists, str): + word_lists = word_lists.split(",") + word_lists = [os.path.join(srcdir, word_list) for word_list in word_lists] + + self._tokenizer_lang = config.tokenizer_lang + self._all_filters = [*cheap_filters, *slow_filters] + self._slow_filters = slow_filters + self._slow_tokenizer: Any = None + self._tokenizer: Any = get_tokenizer(config.tokenizer_lang, filters=cheap_filters) + self._dictionary = enchant.DictWithPWL(config.spelling_lang, _combine_word_lists(word_lists)) + self._cache: dict[str, tuple[tuple[str, int], ...]] = {} + + def find_misspellings(self, text: str) -> Iterator[tuple[str, int]]: + """Yield each misspelled word in ``text`` with its offset.""" + for match in _CHUNK.finditer(text): + for word, offset in self._check_chunk(match[0]): + yield word, match.start() + offset + + def _check_chunk(self, chunk: str) -> tuple[tuple[str, int], ...]: + result = self._cache.get(chunk) + if result is not None: + return result + check = self._dictionary.check + # Filters only ever drop tokens, so a single word the dictionary knows needs no tokenizing. + if chunk.isalpha() and chunk.isascii() and check(chunk): + result = () + else: + result = tuple((word, pos) for word, pos in self._tokenizer(chunk) if not check(word)) + if result and self._slow_filters: + if self._slow_tokenizer is None: + self._slow_tokenizer = get_tokenizer(self._tokenizer_lang, filters=self._all_filters) + result = tuple((word, pos) for word, pos in self._slow_tokenizer(chunk) if not check(word)) + self._cache[chunk] = result + return result + + +def _combine_word_lists(word_lists: list[str]) -> str: + if len(word_lists) == 1: + return word_lists[0] + fd, combined = tempfile.mkstemp(prefix="spelling_wordlist_", suffix=".txt") + with os.fdopen(fd, "w", encoding="utf-8") as out: + for word_list in word_lists: + out.write(Path(word_list).read_text(encoding="utf-8").rstrip("\n") + "\n") + return combined + + +class _SpellingRun: + def __init__(self, app: Sphinx) -> None: + self.checker = SpellChecker(app.config, app.srcdir) + self.excluded = Matcher(app.config.spelling_exclude_patterns) + self.output = Path(app.config.airflow_spelling_output) + self.findings: list[dict[str, Any]] = [] + + def check_document(self, env: BuildEnvironment, docname: str, doctree: nodes.document) -> None: + if self.excluded(str(env.doc2path(docname, False))): + return + good_words = set(getattr(env, "spelling_document_words", {}).get(docname, ())) + for node in doctree.findall(nodes.Text): + parent = node.parent + if parent is not None and parent.tagname not in SpellingBuilder.TEXT_NODES: + continue + text = node.astext().translate(_UNSMARTEN) + for word, offset in self.checker.find_misspellings(text): + if word not in good_words: + self.findings.append(_describe(node, text, word, offset)) + + +def _describe(node: nodes.Text, text: str, word: str, offset: int) -> dict[str, Any]: + line_start = text.rfind("\n", 0, offset) + 1 + line_end = text.find("\n", offset) + context = text[line_start : len(text) if line_end == -1 else line_end].strip() + source, node_line = get_source_line(node) + line = None if node_line is None else node_line + text.count("\n", 0, offset) + return {"file": source, "line": line, "word": word, "context": context} + + +def _start(app: Sphinx) -> None: + if app.config.airflow_spelling_output: + setattr(app, _RUN_ATTRIBUTE, _SpellingRun(app)) + + +def _check(app: Sphinx, doctree: nodes.document, docname: str) -> None: + run: _SpellingRun | None = getattr(app, _RUN_ATTRIBUTE, None) + if run is not None: + run.check_document(app.env, docname, doctree) + + +def _write_findings(app: Sphinx, exception: Exception | None) -> None: + run: _SpellingRun | None = getattr(app, _RUN_ATTRIBUTE, None) + if run is None or exception is not None: + return + run.output.parent.mkdir(parents=True, exist_ok=True) + run.output.write_text(json.dumps(run.findings, indent=2), encoding="utf-8") + + +def spelling_ignore_role( + name: str, + rawtext: str, + text: str, + lineno: int, + inliner: Inliner, + options: dict[str, Any] | None = None, + content: Sequence[str] = (), +) -> tuple[list[nodes.Node], list[nodes.system_message]]: + # sphinxcontrib.spelling flags the Text node itself, but SmartQuotes replaces Text nodes during an + # html build, losing the flag; text inside an inline node is never checked. + return [nodes.inline(rawtext, text, classes=["spelling-ignore"])], [] + + +def setup(app: Sphinx) -> dict[str, Any]: + app.setup_extension("sphinxcontrib.spelling") + app.add_role_to_domain("spelling", "ignore", spelling_ignore_role, override=True) + app.add_config_value("airflow_spelling_output", "", "", types=frozenset({str})) + app.connect("builder-inited", _start) + app.connect("doctree-resolved", _check) + app.connect("build-finished", _write_findings) + return {"version": "builtin", "parallel_read_safe": True, "parallel_write_safe": True} diff --git a/devel-common/src/sphinx_exts/docs_build/docs_builder.py b/devel-common/src/sphinx_exts/docs_build/docs_builder.py index 286635921c137..575980edb1600 100644 --- a/devel-common/src/sphinx_exts/docs_build/docs_builder.py +++ b/devel-common/src/sphinx_exts/docs_build/docs_builder.py @@ -41,7 +41,7 @@ PROCESS_TIMEOUT, ) from sphinx_exts.docs_build.errors import DocBuildError, parse_sphinx_warnings -from sphinx_exts.docs_build.spelling_checks import SpellingError, parse_spelling_warnings +from sphinx_exts.docs_build.spelling_checks import SpellingError, load_spelling_errors console = Console(force_terminal=True, color_system="standard", width=CONSOLE_WIDTH) @@ -211,14 +211,9 @@ def _build_dir(self) -> Path: return GENERATED_PATH / "_build" / "docs" / self.package_name @property - def log_spelling_filename(self) -> Path: - """Log from spelling job.""" - return self._build_dir / f"output-spelling-{self.package_name}.log" - - @property - def log_spelling_output_dir(self) -> Path: - """Results from spelling job.""" - return self._build_dir / f"output-spelling-results-{self.package_name}" + def spelling_output_file(self) -> Path: + """Misspellings found while building, written by the ``airflow_spelling`` extension.""" + return self._build_dir / f"output-spelling-{self.package_name}.json" @property def log_build_filename(self) -> Path: @@ -275,85 +270,16 @@ def clean_files(self) -> None: self._api_dir.mkdir(parents=True, exist_ok=True) self._build_dir.mkdir(parents=True, exist_ok=True) - def check_spelling(self, verbose: bool) -> tuple[list[SpellingError], list[DocBuildError]]: - """ - Checks spelling - - :param verbose: whether to show output while running - :return: list of errors - """ - spelling_errors = [] - build_errors = [] - os.makedirs(self._build_dir, exist_ok=True) - shutil.rmtree(self.log_spelling_output_dir, ignore_errors=True) - self.log_spelling_output_dir.mkdir(parents=True, exist_ok=True) - - command = self.get_command() - build_cmd = [ - command, - "-W", # turn warnings into errors - "--color", # do emit colored output - "-T", # show full traceback on exception - "-b", # builder to use - "spelling", - "-d", # path for the cached environment and doctree files - self._doctree_dir.as_posix(), - # documentation source files - self._src_dir.as_posix(), - self.log_spelling_output_dir.as_posix(), - ] - if os.environ.get("CI", "") != "true" and verbose: - console.print("[yellow]Command to run:[/] ", " ".join([shlex.quote(arg) for arg in build_cmd])) - if verbose: - console.print( - f"[bright_blue]{self.package_name:60}:[/] The output is hidden until an error occurs." - ) - returncode = self._run_sphinx(build_cmd, log_file=self.log_spelling_filename, verbose=verbose) - if returncode != 0: - spelling_errors.append( - SpellingError( - file_path=None, - line_no=None, - spelling=None, - suggestion=None, - context_line=None, - message=f"Sphinx spellcheck returned non-zero exit status: {returncode}.", - ) - ) - spelling_warning_text = "" - for filepath in self.log_spelling_output_dir.rglob("*.spelling"): - with open(filepath) as spelling_file: - spelling_warning_text += spelling_file.read() - spelling_errors.extend(parse_spelling_warnings(spelling_warning_text, self._src_dir)) - if os.path.isfile(self.log_spelling_filename): - with open(self.log_spelling_filename) as warning_file: - warning_text = warning_file.read() - # Remove 7-bit C1 ANSI escape sequences - warning_text = re.sub(r"\x1B[@-_][0-?]*[ -/]*[@-~]", "", warning_text) - build_errors.extend(parse_sphinx_warnings(warning_text, self._src_dir)) - console.print( - f"[bright_blue]{self.package_name:60}:[/] [red]Finished spell-checking with errors[/]" - ) - else: - if spelling_errors: - console.print( - f"[bright_blue]{self.package_name:60}:[/] [yellow]Finished spell-checking with warnings[/]" - ) - else: - console.print( - f"[bright_blue]{self.package_name:60}:[/] [green]Finished spell-checking successfully[/]" - ) - return spelling_errors, build_errors - - def build_sphinx_docs(self, verbose: bool) -> list[DocBuildError]: + def build_sphinx_docs(self, verbose: bool) -> tuple[list[DocBuildError], list[SpellingError]]: """ - Build Sphinx documentation. + Build Sphinx documentation, checking its spelling as it is written. :param verbose: whether to show output while running - :return: list of errors + :return: build errors and spelling errors """ build_errors = [] os.makedirs(self._build_dir, exist_ok=True) + self.spelling_output_file.unlink(missing_ok=True) command = self.get_command() build_cmd = [ command, @@ -365,6 +291,8 @@ def build_sphinx_docs(self, verbose: bool) -> list[DocBuildError]: self._doctree_dir.as_posix(), "-w", # write warnings (and errors) to given file self.log_build_warning_filename.as_posix(), + "-D", + f"airflow_spelling_output={self.spelling_output_file.as_posix()}", # documentation source files self._src_dir.as_posix(), self._build_dir.as_posix(), # path to output directory @@ -390,7 +318,8 @@ def build_sphinx_docs(self, verbose: bool) -> list[DocBuildError]: # Remove 7-bit C1 ANSI escape sequences warning_text = re.sub(r"\x1B[@-_][0-?]*[ -/]*[@-~]", "", warning_text) build_errors.extend(parse_sphinx_warnings(warning_text, self._src_dir)) - if build_errors: + spelling_errors = self._read_spelling_errors(returncode) + if build_errors or spelling_errors: console.print( f"[bright_blue]{self.package_name:60}:[/] [red]Finished docs building with errors[/]" ) @@ -398,7 +327,24 @@ def build_sphinx_docs(self, verbose: bool) -> list[DocBuildError]: console.print( f"[bright_blue]{self.package_name:60}:[/] [green]Finished docs building successfully[/]" ) - return build_errors + return build_errors, spelling_errors + + def _read_spelling_errors(self, returncode: int) -> list[SpellingError]: + if self.spelling_output_file.is_file(): + return load_spelling_errors(self.spelling_output_file) + if returncode != 0: + return [] + return [ + SpellingError( + file_path=None, + line_no=None, + spelling=None, + suggestion=None, + context_line=None, + message=f"Spelling was not checked: {self.spelling_output_file} was not written. " + "Make sure the documentation's conf.py enables the airflow_spelling extension.", + ) + ] def get_command(self) -> str: return "sphinx-autobuild" if self.is_autobuild else "sphinx-build" diff --git a/devel-common/src/sphinx_exts/docs_build/spelling_checks.py b/devel-common/src/sphinx_exts/docs_build/spelling_checks.py index 94a2de20500ed..06d541299142f 100644 --- a/devel-common/src/sphinx_exts/docs_build/spelling_checks.py +++ b/devel-common/src/sphinx_exts/docs_build/spelling_checks.py @@ -16,8 +16,8 @@ # under the License. from __future__ import annotations +import json import os -import re from functools import total_ordering from pathlib import Path from typing import NamedTuple @@ -25,7 +25,7 @@ from rich.console import Console from airflow.utils.code_utils import prepare_code_snippet -from sphinx_exts.docs_build.code_utils import CONSOLE_WIDTH +from sphinx_exts.docs_build.code_utils import AIRFLOW_CONTENT_ROOT_PATH, CONSOLE_WIDTH CURRENT_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__))) DOCS_DIR = os.path.abspath(os.path.join(CURRENT_DIR, os.pardir, os.pardir)) @@ -91,58 +91,67 @@ def __lt__(self, other): return left < right -def parse_spelling_warnings(warning_text: str, docs_dir: Path) -> list[SpellingError]: - """ - Parses warnings from Sphinx. - - :param warning_text: warning to parse - :param docs_dir: documentation directory - :return: list of SpellingError. - """ - sphinx_spelling_errors = [] - for sphinx_warning in warning_text.splitlines(): - if not sphinx_warning: - continue - warning_parts = None - match = re.search(r"(.*):(\w*):\s\((\w*)\)\s?(\w*)\s?(.*)", sphinx_warning) - if match: - warning_parts = match.groups() - if warning_parts and len(warning_parts) == 5: - try: - sphinx_spelling_errors.append( - SpellingError( - file_path=docs_dir / warning_parts[0], - line_no=int(warning_parts[1]) if warning_parts[1] not in ("None", "") else None, - spelling=warning_parts[2], - suggestion=warning_parts[3] if warning_parts[3] else None, - context_line=warning_parts[4], - message=sphinx_warning, - ) - ) - except Exception: - # If an exception occurred while parsing the warning message, display the raw warning message. - sphinx_spelling_errors.append( - SpellingError( - file_path=None, - line_no=None, - spelling=None, - suggestion=None, - context_line=None, - message=sphinx_warning, - ) - ) - else: - sphinx_spelling_errors.append( - SpellingError( - file_path=None, - line_no=None, - spelling=None, - suggestion=None, - context_line=None, - message=sphinx_warning, - ) +def load_spelling_errors(path: Path) -> list[SpellingError]: + """Load the misspellings the ``airflow_spelling`` extension wrote during a build.""" + errors = [] + for finding in json.loads(path.read_text(encoding="utf-8")): + file_path = Path(finding["file"]) if finding["file"] else None + location = _relative_to_repo(file_path) if file_path else "" + errors.append( + SpellingError( + file_path=file_path, + line_no=finding["line"], + spelling=finding["word"], + suggestion=None, + context_line=finding["context"], + message=f"{location}:{finding['line']}: ({finding['word']}) {finding['context']}", + ) + ) + return errors + + +def _relative_to_repo(path: Path) -> str: + return ( + path.relative_to(AIRFLOW_CONTENT_ROOT_PATH).as_posix() + if path.is_relative_to(AIRFLOW_CONTENT_ROOT_PATH) + else path.as_posix() + ) + + +def _escape_annotation_property(value: str) -> str: + return ( + value.replace("%", "%25") + .replace("\r", "%0D") + .replace("\n", "%0A") + .replace(":", "%3A") + .replace(",", "%2C") + ) + + +def _escape_annotation_message(value: str) -> str: + return value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def emit_github_annotations(spelling_errors: dict[str, list[SpellingError]]) -> None: + """Print a GitHub Actions error annotation per misspelling, so it is shown on the line in the PR diff.""" + if os.environ.get("GITHUB_ACTIONS") != "true": + return + for errors in spelling_errors.values(): + for error in sorted(errors): + if error.file_path is None or error.spelling is None or not error.file_path.is_file(): + continue + # Pages autoapi generates under _api/ are gitignored, so GitHub has no file to show the error on. + if "_api" in error.file_path.parts: + continue + properties = f"file={_escape_annotation_property(_relative_to_repo(error.file_path))}" + if error.line_no: + properties += f",line={error.line_no}" + properties += ",title=Spelling" + message = ( + f"Unknown word '{error.spelling}' in: {error.context_line}. If it is spelled correctly, " + "quote code in backticks or add the word to docs/spelling_wordlist.txt." ) - return sphinx_spelling_errors + print(f"::error {properties}::{_escape_annotation_message(message)}", flush=True) def display_spelling_error_summary(spelling_errors: dict[str, list[SpellingError]]) -> None: diff --git a/devel-common/tests/unit/sphinx_exts/docs_build/test_docs_builder.py b/devel-common/tests/unit/sphinx_exts/docs_build/test_docs_builder.py index a432cb3715e6f..5c01febc94180 100644 --- a/devel-common/tests/unit/sphinx_exts/docs_build/test_docs_builder.py +++ b/devel-common/tests/unit/sphinx_exts/docs_build/test_docs_builder.py @@ -17,12 +17,16 @@ from __future__ import annotations import io +import json import os import subprocess import sys import types +from pathlib import Path from unittest import mock +import pytest + from sphinx_exts.docs_build import docs_builder from sphinx_exts.docs_build.code_utils import AIRFLOW_CONTENT_ROOT_PATH, DOCS_SOURCES_PATH from sphinx_exts.docs_build.docs_builder import ( @@ -155,3 +159,51 @@ def test_autobuild_stays_a_subprocess(self, tmp_path, monkeypatch): run.assert_called_once() assert run.call_args.args[0] == ["sphinx-autobuild", "src", "out"] docs_builder.build_main.assert_not_called() + + +class TestBuildSphinxDocsSpelling: + @pytest.fixture + def builder(self, tmp_path, monkeypatch): + monkeypatch.setattr(docs_builder, "GENERATED_PATH", tmp_path) + return AirflowDocsBuilder(package_name="apache-airflow-providers-ftp") + + @staticmethod + def _fake_run_sphinx(findings: list[dict] | None, returncode: int = 0): + def run_sphinx(self, build_cmd, *, log_file, verbose): + spelling_args = [arg for arg in build_cmd if arg.startswith("airflow_spelling_output=")] + if findings is not None and spelling_args: + output = Path(spelling_args[0].split("=", 1)[1]) + output.write_text(json.dumps(findings)) + return returncode + + return run_sphinx + + def test_reports_the_misspellings_written_during_the_build(self, builder, monkeypatch): + findings = [{"file": "/docs/index.rst", "line": 3, "word": "zorp", "context": "a zorp"}] + monkeypatch.setattr(AirflowDocsBuilder, "_run_sphinx", self._fake_run_sphinx(findings)) + + build_errors, spelling_errors = builder.build_sphinx_docs(verbose=False) + + assert build_errors == [] + assert [(e.file_path, e.line_no, e.spelling) for e in spelling_errors] == [ + (Path("/docs/index.rst"), 3, "zorp") + ] + + def test_a_successful_build_without_spelling_results_is_an_error(self, builder, monkeypatch): + monkeypatch.setattr(AirflowDocsBuilder, "_run_sphinx", self._fake_run_sphinx(None)) + + _, spelling_errors = builder.build_sphinx_docs(verbose=False) + + assert len(spelling_errors) == 1 + assert spelling_errors[0].message.startswith("Spelling was not checked") + + def test_stale_results_from_an_earlier_build_are_not_reported(self, builder, monkeypatch): + builder.spelling_output_file.parent.mkdir(parents=True) + builder.spelling_output_file.write_text( + json.dumps([{"file": None, "line": None, "word": "x", "context": ""}]) + ) + monkeypatch.setattr(AirflowDocsBuilder, "_run_sphinx", self._fake_run_sphinx(None, returncode=2)) + + _, spelling_errors = builder.build_sphinx_docs(verbose=False) + + assert spelling_errors == [] diff --git a/devel-common/tests/unit/sphinx_exts/docs_build/test_spelling_checks.py b/devel-common/tests/unit/sphinx_exts/docs_build/test_spelling_checks.py new file mode 100644 index 0000000000000..3e20479f0fe13 --- /dev/null +++ b/devel-common/tests/unit/sphinx_exts/docs_build/test_spelling_checks.py @@ -0,0 +1,134 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json + +import pytest + +from sphinx_exts.docs_build.code_utils import AIRFLOW_CONTENT_ROOT_PATH +from sphinx_exts.docs_build.spelling_checks import ( + SpellingError, + emit_github_annotations, + load_spelling_errors, +) + +SOURCE = AIRFLOW_CONTENT_ROOT_PATH / "task-sdk" / "src" / "airflow" / "sdk" / "importers" / "base.py" + + +def test_load_spelling_errors(tmp_path): + output = tmp_path / "output-spelling.json" + output.write_text( + json.dumps( + [ + {"file": SOURCE.as_posix(), "line": 86, "word": "routable", "context": "source routable"}, + {"file": None, "line": None, "word": "zorp", "context": "a zorp"}, + ] + ) + ) + + errors = load_spelling_errors(output) + + assert errors == [ + SpellingError( + file_path=SOURCE, + line_no=86, + spelling="routable", + suggestion=None, + context_line="source routable", + message="task-sdk/src/airflow/sdk/importers/base.py:86: (routable) source routable", + ), + SpellingError( + file_path=None, + line_no=None, + spelling="zorp", + suggestion=None, + context_line="a zorp", + message=":None: (zorp) a zorp", + ), + ] + + +class TestEmitGithubAnnotations: + ERRORS = { + "task-sdk": [ + SpellingError( + file_path=SOURCE, + line_no=86, + spelling="routable", + suggestion=None, + context_line="50% done, really: yes", + message="", + ), + SpellingError( + file_path=SOURCE.with_name("base.py:docstring of airflow.sdk.FileDagDefinition"), + line_no=4, + spelling="routable", + suggestion=None, + context_line="source routable", + message="", + ), + SpellingError( + file_path=None, + line_no=None, + spelling=None, + suggestion=None, + context_line=None, + message="Spelling was not checked", + ), + ] + } + + def test_prints_nothing_for_pages_generated_by_autoapi(self, monkeypatch, capsys, tmp_path): + generated_page = tmp_path / "docs" / "_api" / "airflow" / "sdk" / "index.rst" + generated_page.parent.mkdir(parents=True) + generated_page.write_text("generated") + monkeypatch.setenv("GITHUB_ACTIONS", "true") + error = SpellingError( + file_path=generated_page, + line_no=12, + spelling="routable", + suggestion=None, + context_line="source routable", + message="", + ) + + emit_github_annotations({"task-sdk": [error]}) + + assert capsys.readouterr().out == "" + + def test_prints_one_error_annotation_per_misspelling_in_a_real_file(self, monkeypatch, capsys): + monkeypatch.setenv("GITHUB_ACTIONS", "true") + + emit_github_annotations(self.ERRORS) + + assert capsys.readouterr().out == ( + "::error file=task-sdk/src/airflow/sdk/importers/base.py,line=86,title=Spelling::" + "Unknown word 'routable' in: 50%25 done, really: yes. If it is spelled correctly, " + "quote code in backticks or add the word to docs/spelling_wordlist.txt.\n" + ) + + @pytest.mark.parametrize("github_actions", [None, "false"]) + def test_prints_nothing_outside_github_actions(self, monkeypatch, capsys, github_actions): + if github_actions is None: + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + else: + monkeypatch.setenv("GITHUB_ACTIONS", github_actions) + + emit_github_annotations(self.ERRORS) + + assert capsys.readouterr().out == "" diff --git a/devel-common/tests/unit/sphinx_exts/test_airflow_intersphinx.py b/devel-common/tests/unit/sphinx_exts/test_airflow_intersphinx.py index ce5b77ad9ba0c..af886aac1f1c9 100644 --- a/devel-common/tests/unit/sphinx_exts/test_airflow_intersphinx.py +++ b/devel-common/tests/unit/sphinx_exts/test_airflow_intersphinx.py @@ -18,13 +18,8 @@ import sys from pathlib import Path -from unittest import mock import pytest -from sphinx.application import Sphinx -from sphinx.builders import Builder -from sphinx.environment import BuildEnvironment -from sphinx.errors import SphinxError SPHINX_EXTS_PATH = Path(__file__).parents[3] / "src" / "sphinx_exts" if SPHINX_EXTS_PATH.as_posix() not in sys.path: @@ -52,18 +47,10 @@ class TestInventoryFor: def test_prefers_the_html_inventory_built_in_this_run(self, generated_path): build_dir = generated_path / "_build" / "docs" / PACKAGE / "stable" html = _touch(build_dir / "objects.inv") - _touch(build_dir / f"output-spelling-results-{PACKAGE}" / "objects.inv") _touch(generated_path / "_inventory_cache" / PACKAGE / "objects.inv") assert airflow_intersphinx._inventory_for(PACKAGE, versioned=True) == html - def test_uses_the_spelling_inventory_when_only_a_spelling_build_ran(self, generated_path): - build_dir = generated_path / "_build" / "docs" / PACKAGE / "stable" - spelling = _touch(build_dir / f"output-spelling-results-{PACKAGE}" / "objects.inv") - _touch(generated_path / "_inventory_cache" / PACKAGE / "objects.inv") - - assert airflow_intersphinx._inventory_for(PACKAGE, versioned=True) == spelling - def test_falls_back_to_the_downloaded_inventory(self, generated_path): cached = generated_path / "_inventory_cache" / PACKAGE / "objects.inv" @@ -73,41 +60,3 @@ def test_non_versioned_packages_have_no_stable_directory(self, generated_path): html = _touch(generated_path / "_build" / "docs" / "docker-stack" / "objects.inv") assert airflow_intersphinx._inventory_for("docker-stack", versioned=False) == html - - -class TestDumpInventoryAfterSpellingBuild: - @staticmethod - def _app(builder_name: str, outdir: Path) -> mock.Mock: - app = mock.Mock(spec=Sphinx) - app.builder = mock.Mock(spec=Builder) - app.builder.name = builder_name - app.outdir = outdir - app.env = mock.Mock(spec=BuildEnvironment) - return app - - @mock.patch.object(airflow_intersphinx.InventoryFile, "dump", autospec=True) - def test_writes_an_inventory_into_the_spelling_output_dir(self, mock_dump, tmp_path): - app = self._app("spelling", tmp_path) - - airflow_intersphinx._dump_inventory_after_spelling_build(app, None) - - mock_dump.assert_called_once() - filename, env, uri_builder = mock_dump.call_args.args - assert filename == (tmp_path / "objects.inv").as_posix() - assert env is app.env - assert uri_builder.get_target_uri("operators/index") == "operators/index.html" - - @pytest.mark.parametrize( - ("builder_name", "exception"), - [ - pytest.param("html", None, id="html-builder-writes-its-own"), - pytest.param("spelling", SphinxError("boom"), id="failed-build"), - ], - ) - @mock.patch.object(airflow_intersphinx.InventoryFile, "dump", autospec=True) - def test_skips_other_builders_and_failed_builds(self, mock_dump, tmp_path, builder_name, exception): - app = self._app(builder_name, tmp_path) - - airflow_intersphinx._dump_inventory_after_spelling_build(app, exception) - - mock_dump.assert_not_called() diff --git a/devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py b/devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py new file mode 100644 index 0000000000000..c03ce08344884 --- /dev/null +++ b/devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py @@ -0,0 +1,213 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json +import sys +from pathlib import Path +from types import SimpleNamespace +from unittest import mock + +import enchant +import pytest +from docutils import nodes +from docutils.utils import new_document +from docutils.utils.smartquotes import smartchars, smartyPants +from sphinx.application import Sphinx +from sphinx.environment import BuildEnvironment +from sphinx.errors import SphinxError +from sphinxcontrib.spelling import filters + +SPHINX_EXTS_PATH = Path(__file__).parents[3] / "src" / "sphinx_exts" +if SPHINX_EXTS_PATH.as_posix() not in sys.path: + # The extensions are loaded by Sphinx from this directory and import each other by bare name. + sys.path.append(SPHINX_EXTS_PATH.as_posix()) + +from sphinx_exts import airflow_spelling # noqa: E402 + +KNOWN_WORDS = {"the", "word", "is", "spelled", "do", "Dag", "text", "quoted", "a"} + + +def _config(**overrides): + values = { + "spelling_ignore_wiki_words": True, + "spelling_ignore_acronyms": True, + "spelling_ignore_python_builtins": True, + "spelling_ignore_importable_modules": True, + "spelling_ignore_contributor_names": False, + "spelling_filters": [], + "spelling_word_list_filename": ["wordlist.txt"], + "spelling_lang": "en_US", + "tokenizer_lang": "en_US", + "spelling_exclude_patterns": [], + "airflow_spelling_output": "", + } + values.update(overrides) + return SimpleNamespace(**values) + + +@pytest.fixture(autouse=True) +def dictionary_factory(monkeypatch): + pwl = mock.create_autospec(enchant.DictWithPWL, instance=True) + pwl.check.side_effect = KNOWN_WORDS.__contains__ + factory = mock.create_autospec(enchant.DictWithPWL, return_value=pwl) + monkeypatch.setattr(airflow_spelling.enchant, "DictWithPWL", factory) + return factory + + +@pytest.fixture +def dictionary(dictionary_factory): + return dictionary_factory.return_value + + +@pytest.fixture +def checker(tmp_path): + return airflow_spelling.SpellChecker(_config(), tmp_path) + + +class TestSpellChecker: + def test_reports_unknown_words_with_their_offset_in_the_text(self, checker): + text = "the zorp is\nspelled (badly) Dag's" + + assert list(checker.find_misspellings(text)) == [("zorp", 4), ("badly", 21)] + + def test_skips_what_the_spelling_builder_filters_skip(self, checker): + text = "API URLs WikiWord len someone@example.com sys don't" + + assert list(checker.find_misspellings(text)) == [] + + def test_each_distinct_chunk_is_checked_once(self, checker, dictionary): + list(checker.find_misspellings("zorp")) + checks_for_first_sight = dictionary.check.call_count + + assert list(checker.find_misspellings("zorp zorp")) == [("zorp", 0), ("zorp", 5)] + assert dictionary.check.call_count == checks_for_first_sight + + @mock.patch.object(filters.ImportableModuleFilter, "_skip", autospec=True, return_value=False) + def test_importable_module_lookup_only_runs_for_words_still_misspelled(self, mock_skip, checker): + list(checker.find_misspellings("the zorp")) + + assert [c.args[1] for c in mock_skip.call_args_list] == ["zorp"] + + def test_combines_several_word_lists(self, tmp_path, dictionary_factory): + (tmp_path / "a.txt").write_text("alpha\n") + (tmp_path / "b.txt").write_text("beta") + + airflow_spelling.SpellChecker(_config(spelling_word_list_filename=["a.txt", "b.txt"]), tmp_path) + + combined = dictionary_factory.call_args.args[1] + assert Path(combined).read_text() == "alpha\nbeta\n" + + +def test_undoes_every_character_smartquotes_produces(): + produced = { + value + for name, value in vars(smartchars).items() + if isinstance(value, str) and not name.startswith("_") + } + produced.update(smartchars.quotes["en"]) + + assert {character for character in produced if ord(character) not in airflow_spelling._UNSMARTEN} == set() + + +@pytest.mark.parametrize( + "typed", + ["the \"double\" and 'single' quotes", "don't, the '80s", "an -- en and --- em dash", "and so on..."], +) +def test_undoing_smartquotes_gives_back_the_text_as_typed(typed): + assert smartyPants(typed, "qDe", "en").translate(airflow_spelling._UNSMARTEN) == typed + + +def _env(docname_path: str = "index.rst", good_words=None): + env = mock.create_autospec(BuildEnvironment, instance=True) + env.doc2path.return_value = Path(docname_path) + env.spelling_document_words = {"index": good_words or []} + return env + + +def _paragraph(*children: nodes.Node, source: str, line: int) -> nodes.document: + document = new_document(source) + paragraph = nodes.paragraph("", "", *children) + paragraph.source = source + paragraph.line = line + document += paragraph + return document + + +class TestCheckDocument: + def _run(self, tmp_path, **config): + app = mock.create_autospec(Sphinx, instance=True) + app.config = _config(airflow_spelling_output=(tmp_path / "out.json").as_posix(), **config) + app.srcdir = tmp_path + return airflow_spelling._SpellingRun(app) + + def test_checks_smart_quoted_paragraph_text_but_not_literals_ignored_text_or_document_words( + self, tmp_path + ): + run = self._run(tmp_path) + doctree = _paragraph( + nodes.Text("the zorp\ndon’t “quoted” ‘WikiWord’ “ZorpQuux” isinstance… mispeled localword "), + nodes.literal("", "notchecked"), + *airflow_spelling.spelling_ignore_role("", "", "ignoredword", 1, mock.Mock())[0], + source="/docs/index.rst", + line=3, + ) + + run.check_document(_env(good_words=["localword"]), "index", doctree) + + assert run.findings == [ + {"file": "/docs/index.rst", "line": 3, "word": "zorp", "context": "the zorp"}, + { + "file": "/docs/index.rst", + "line": 4, + "word": "mispeled", + "context": """don't "quoted" 'WikiWord' "ZorpQuux" isinstance... mispeled localword""", + }, + ] + + def test_skips_excluded_documents(self, tmp_path): + run = self._run(tmp_path, spelling_exclude_patterns=["changelog.rst"]) + doctree = _paragraph(nodes.Text("zorp"), source="/docs/changelog.rst", line=1) + + run.check_document(_env("changelog.rst"), "changelog", doctree) + + assert run.findings == [] + + +class TestWriteFindings: + def _app(self, tmp_path, findings): + app = mock.create_autospec(Sphinx, instance=True) + run = mock.create_autospec(airflow_spelling._SpellingRun, instance=True) + run.output = tmp_path / "nested" / "out.json" + run.findings = findings + setattr(app, airflow_spelling._RUN_ATTRIBUTE, run) + return app, run.output + + def test_writes_findings_as_json(self, tmp_path): + findings = [{"file": "/docs/index.rst", "line": 1, "word": "zorp", "context": "zorp"}] + app, output = self._app(tmp_path, findings) + + airflow_spelling._write_findings(app, None) + + assert json.loads(output.read_text()) == findings + + def test_writes_nothing_when_the_build_failed(self, tmp_path): + app, output = self._app(tmp_path, []) + + airflow_spelling._write_findings(app, SphinxError("boom")) + + assert not output.exists() diff --git a/docs/README.md b/docs/README.md index 02c4af4b5d85a..21533eec881ea 100644 --- a/docs/README.md +++ b/docs/README.md @@ -284,7 +284,7 @@ Note that it is advised to add `--dry-run` if you just want to see what would ha the `s3://staging-docs-airflow-apache-org/docs/` bucket to test the publishing using staging site. ```bash -breeze build-docs "" "" --docs-only +breeze build-docs "" "" mkdir /tmp/airflow-site breeze release-management publish-docs --override-versioned --airflow-site-directory /tmp/airflow-site breeze release-management publish-docs-to-s3 --source-dir-path /tmp/airflow-site/docs-archive \ @@ -308,7 +308,7 @@ The process is as follows: publish the docs to the checked out `airflow-site-archive` branch. ```bash -breeze build-docs "" "" --docs-only +breeze build-docs "" "" breeze release-management publish-docs --override-versioned --airflow-site-directory ``` diff --git a/task-sdk/docs/conf.py b/task-sdk/docs/conf.py index c53f14b63b1fc..3b9c6f17eeeb8 100644 --- a/task-sdk/docs/conf.py +++ b/task-sdk/docs/conf.py @@ -58,7 +58,7 @@ "airflow_intersphinx", "exampleinclude", "sphinxcontrib.mermaid", - "sphinxcontrib.spelling", + "airflow_spelling", ] autoapi_dirs = [CONF_DIR.joinpath("..", "src").resolve()] From 7aaf9259b6cc61a8cb1aef7c6c465a7fe8e83493 Mon Sep 17 00:00:00 2001 From: Ash Berlin-Taylor Date: Mon, 12 Oct 2026 15:44:22 +0100 Subject: [PATCH 2/3] fixup! Halve the time to build docs locally by inlining spell checking --- chart/pyproject.toml | 6 +----- contributing-docs/11_documentation_building.rst | 10 +++++----- devel-common/src/docs/README.rst | 2 +- docker-stack-docs/pyproject.toml | 6 +----- providers-summary-docs/pyproject.toml | 6 +----- 5 files changed, 9 insertions(+), 21 deletions(-) diff --git a/chart/pyproject.toml b/chart/pyproject.toml index 67b8d2f0b7da3..500dfb78ea979 100644 --- a/chart/pyproject.toml +++ b/chart/pyproject.toml @@ -98,14 +98,10 @@ exclude = ["*"] bypass-selection = true [dependency-groups] -# To build docs run: +# To build docs and check their spelling run: # # uv run --group docs sphinx-build -T --color -b html . _build # -# To check spelling: -# -# uv run --group docs sphinx-build -T --color -b spelling . _build -# # To enable auto-refreshing build with server: # # uv run --group docs sphinx-autobuild -T --color -b html . _build diff --git a/contributing-docs/11_documentation_building.rst b/contributing-docs/11_documentation_building.rst index 6188906412b3f..1297b1048b8ca 100644 --- a/contributing-docs/11_documentation_building.rst +++ b/contributing-docs/11_documentation_building.rst @@ -89,7 +89,7 @@ You should run ``uv python pin 3.11`` to pin to Python 3.11 in order to build do .. code-block:: txt - Could not import extension sphinxcontrib.spelling (exception: The 'enchant' C library was not + Could not import extension airflow_spelling (exception: The 'enchant' C library was not found and maybe needs to be installed The easiest way is to find where your enchant library is (usually libenchant-2.dylib) @@ -229,15 +229,15 @@ and it is independent on the host / local environment including the OS you are u fall-back to this method if - for some reason - your local documentation building is failing. Basic command is ``breeze build-docs`` which except ``--autobuild`` has very similar options -as the ``uv run build-docs`` command.. You can also specify a number of options like selecting which -distribution packages you want to build and which kind of build to run (``--doc-only`` or ``--spelling-only``) -or request to ``--clean`` the build directory before building the documentation. +as the ``uv run build-docs`` command. Spelling is checked as part of every build. You can also specify a +number of options like selecting which distribution packages you want to build or request to +``--clean-build`` the build directory before building the documentation. For example: .. code-block:: bash - breeze build-docs --doc-only --clean fab + breeze build-docs --clean-build fab Will build ``fab`` provider documentation and clean build artifacts before. diff --git a/devel-common/src/docs/README.rst b/devel-common/src/docs/README.rst index 25aa5b664124e..2d021da3af2d0 100644 --- a/devel-common/src/docs/README.rst +++ b/devel-common/src/docs/README.rst @@ -40,7 +40,7 @@ When you make changes to the docs, it follows roughly the same process as creati 1. **Decide to edit in GitHub UI or locally** - Depending on the size and type of docs update you want to make, it might be easier to work in the UI or to make your edits in a local fork. 2. **Find the source files to edit** - While you can access most of the docs source files using the **Suggest a change on this page** button or by searching for a string in the ``/docs/`` file directory, in some cases, the source strings might be located in different provider docs or in the source code itself. -3. **If editing locally, run spellcheck and the build to identify any blocking errors** - Docs require build, spellcheck, and precommit CI/CD tests to pass before they can merge. This means that if you have a pull request with docs changes, a docs build error can prevent your code merge. Checking the build and spelling locally first can help speed up reviews. If you made formatting changes, checking a local build of your docs allows you to make sure you correctly formatted elements like tables, text styling, and line breaks. +3. **If editing locally, run the docs build to identify any blocking errors** - The build also checks spelling. Docs require the build and prek CI/CD checks to pass before they can merge. This means that if you have a pull request with docs changes, a docs build error can prevent your code merge. Checking the build and spelling locally first can help speed up reviews. If you made formatting changes, checking a local build of your docs allows you to make sure you correctly formatted elements like tables, text styling, and line breaks. 4. **Make your pull request** - When you make a PR, GitHub automatically assigns reviewers and runs CI/CD tests. 5. **Fix any build errors or spelling mistakes** - Your PR can't be merged if there are any spelling or build errors. Check to see which builds are failing and click **Show details**. The output of the tests share the errors, location of the problems, and suggest resolutions. Common Docs failures occur due to incorrect formatting and whitespace. diff --git a/docker-stack-docs/pyproject.toml b/docker-stack-docs/pyproject.toml index c8cce5976f6cd..482b3fa3f6ba9 100644 --- a/docker-stack-docs/pyproject.toml +++ b/docker-stack-docs/pyproject.toml @@ -66,14 +66,10 @@ exclude = ["*"] bypass-selection = true [dependency-groups] -# To build docs run: +# To build docs and check their spelling run: # # uv run --group docs sphinx-build -T --color -b html . _build # -# To check spelling: -# -# uv run --group docs sphinx-build -T --color -b spelling . _build -# # To enable auto-refreshing build with server: # # uv run --group docs sphinx-autobuild -T --color -b html . _build diff --git a/providers-summary-docs/pyproject.toml b/providers-summary-docs/pyproject.toml index 5a26b28969dc0..34607588f7359 100644 --- a/providers-summary-docs/pyproject.toml +++ b/providers-summary-docs/pyproject.toml @@ -65,14 +65,10 @@ exclude = ["*"] bypass-selection = true [dependency-groups] -# To build docs run: +# To build docs and check their spelling run: # # uv run --group docs sphinx-build -T --color -b html . _build # -# To check spelling: -# -# uv run --group docs sphinx-build -T --color -b spelling . _build -# # To enable auto-refreshing build with server: # # uv run --group docs sphinx-autobuild -T --color -b html . _build From 3d88045aea349d57eae35705b30c65fff6711f92 Mon Sep 17 00:00:00 2001 From: Ash Berlin-Taylor Date: Mon, 12 Oct 2026 15:59:08 +0100 Subject: [PATCH 3/3] fixup! Halve the time to build docs locally by inlining spell checking --- devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py b/devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py index c03ce08344884..34f6644df8662 100644 --- a/devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py +++ b/devel-common/tests/unit/sphinx_exts/test_airflow_spelling.py @@ -99,7 +99,7 @@ def test_each_distinct_chunk_is_checked_once(self, checker, dictionary): @mock.patch.object(filters.ImportableModuleFilter, "_skip", autospec=True, return_value=False) def test_importable_module_lookup_only_runs_for_words_still_misspelled(self, mock_skip, checker): - list(checker.find_misspellings("the zorp")) + list(checker.find_misspellings("(the) zorp")) assert [c.args[1] for c in mock_skip.call_args_list] == ["zorp"]