Skip to content

feat(rules): add Dhall review rules - #963

Open
chethanuk wants to merge 1 commit into
alibaba:mainfrom
chethanuk:wf/dhall-language-support
Open

feat(rules): add Dhall review rules#963
chethanuk wants to merge 1 commit into
alibaba:mainfrom
chethanuk:wf/dhall-language-support

Conversation

@chethanuk

Copy link
Copy Markdown
Contributor

Description

.dhall files were never reviewed. The extension is missing from supported_file_types.json, so IsAllowedExt drops them at the filter gate before any rule can run, which is why a rule doc on its own would never have been reachable.

This adds .dhall to the allowlist and registers one rule doc in path_rule_map: **/*.dhalldhall.md. Data only, no Go source changes.

The headline check is a remote import by URL with no trailing sha256:<hex> integrity hash. Dhall resolves those over the network when the file is evaluated, so an unpinned import hands whoever controls that URL control of the configuration, and the content it returns can change between two evaluations of the same file. dhall freeze writes the hash.

Two carve-outs keep that check usable. The Prelude's missing sha256:<hex> ? ./local.dhall idiom is a content-addressed cache lookup with a working local fallback rather than an unpinned import, and it sits at the top of nearly every file that imports the Prelude. Bare relative imports (./types.dhall) are the overwhelming majority of imports in a project and pinning them is not the convention. The totality section carves out Natural/fold and List/fold for the same reason: folds are how iteration is written in a language with no recursion, so flagging them would fire on almost every file.

No exclude patterns are added. IsExcludedPath applies every pattern to every path with no language dispatch, so a pattern is only worth its repo-wide reach when it matches paths that are reliably test-only or generated. Dhall has neither in tree. Its import cache lives under $XDG_CACHE_HOME/dhall, outside the repository, so it never appears in a diff, and there is no vendored dependency directory or build output to skip.

Limitation

.dhall diffs that used to be skipped are now reviewed. Anyone who does not want that opts out with user exclude globs, which are checked before the allowlist.

Type of Change

  • New feature (non-breaking change that adds functionality)

How Has This Been Tested?

  • make test passes locally
  • Manual testing (describe below)

Rows appended to the existing table tests:

  • TestIsAllowedExt.dhall and .DHALL.
  • TestResolve_DefaultRulesconfig/prod.dhall and schemas/package.dhall resolve by a heading anchor unique to dhall.md.

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly (if applicable)
  • I have signed the CLA

The rule table in pages/src/content/docs/{en,ja,ru,zh}/review-rules.md lists the new pattern.

Related Issues

Part of #470.

@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s).

@chethanuk
chethanuk force-pushed the wf/dhall-language-support branch from 4bead34 to e685498 Compare August 17, 2026 06:19
@chethanuk

Copy link
Copy Markdown
Contributor Author

@lizhengfeng101 Please review and merge this. Future main updates might cause more conflicts. Thanks :)

@chethanuk
chethanuk force-pushed the wf/dhall-language-support branch from e685498 to 611b2cd Compare August 17, 2026 07:35
@chethanuk
chethanuk force-pushed the wf/dhall-language-support branch from 611b2cd to 848246c Compare August 17, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants