Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ jobs:
name: Code check and tests
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Setup node
uses: actions/setup-node@v4
with:
Expand All @@ -19,6 +21,15 @@ jobs:
run: npx prettier --list-different "src/**/*.[jt]s" "tests/**/*.[jt]s" "src/**/*.vue"
- name: Lint
run: npm run lint
- name: Quality ratchets
# A file may not get worse than it is on the pull request's target branch.
if: github.event_name == 'pull_request'
env:
BASE_REF: ${{ github.base_ref }}
run: |
git fetch --no-tags origin \
"+refs/heads/$BASE_REF:refs/remotes/origin/$BASE_REF"
node scripts/checks/check.mjs --base "origin/$BASE_REF"
- name: Tests
run: npm test

Expand Down
12 changes: 12 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,18 @@ npm run lint

---

## Quality checks

The pre-commit hook and CI run `node scripts/checks/check.mjs`. It compares the files you changed with the base (`HEAD` locally, the target branch in CI), so existing code is left alone and only a change that makes a file worse fails.

- Complexity: in `src`, a function stays within cyclomatic complexity 10, nesting depth 3 and 4 parameters, and a file within 600 lines. A file already over a limit may not go further over, and a new file must be within all of them.
- Duplication: a change to the tests may not add a block of 60 or more tokens that repeats another test.
- Conventions: no new binary files outside the directories that already hold them, such as `tests/baseline`.

The limits are in `scripts/checks/config.mjs`. Stage your changes before running the script by hand, since it reads the index. `CHECKS_SKIP=1 git commit` skips it for one commit.

---

## Testing

```bash
Expand Down
180 changes: 144 additions & 36 deletions eslint.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import path from 'node:path';
import js from '@eslint/js';
import eslintPluginVue from 'eslint-plugin-vue';
import tseslint from 'typescript-eslint';
Expand All @@ -21,15 +22,58 @@ import globals from 'globals';
// the full pattern set its files need, and no block silently erases another
// feature's boundary.
// ---------------------------------------------------------------------------
// `pure.upperModules` is a hand-maintained list of the feature's non-pure
// modules: a new one has to be added here or the pure layer may import it.
// `pure` lists the feature's pure files, and that one list is also everything
// a pure file may import from its own feature: a module that is not on it is
// denied without having to be named. A directory glob admits the directory.
const pureModules = (feature) =>
feature.pure.map((file) =>
file
.slice(`src/${feature.dir}/`.length)
.replace(/\/\*\*\/.*$/, '/**')
.replace(/\.\w+$/, '')
);

// Patterns follow gitignore, which cannot re-admit a path under a denied
// directory. So each directory on the way to a pure module denies its children
// and re-admits the ones that are pure or lead to one.
const ownFeatureExceptPure = (feature) => {
const root = `@/src/${feature.dir}`;
const admitted = new Set(
pureModules(feature).flatMap((module) => {
const parts = module.replace(/\/\*\*$/, '').split('/');
return parts.map((_, depth) => parts.slice(0, depth + 1).join('/'));
})
);
const wholeDirs = pureModules(feature)
.filter((module) => module.endsWith('/**'))
.map((module) => module.slice(0, -3));
const parentOf = (entry) => path.posix.dirname(entry).replace(/^\.$/, '');
const denyingDirs = new Set(
[...admitted].map(parentOf).filter((dir) => !wholeDirs.includes(dir))
);
return [...denyingDirs]
.sort()
.flatMap((dir) => [
`${root}/${dir ? `${dir}/` : ''}*`,
...[...admitted]
.filter((entry) => parentOf(entry) === dir)
.map((entry) => `!${root}/${entry}`),
]);
};

const upperLayerMessage = (feature) =>
`The ${feature.dir} pure layer must not import stores, components, or upper feature modules. Dependencies point downward only.`;

const featureBoundaries = (features) => {
const publicSurface = ({ dir }) => ({
group: [`@/src/${dir}/*`, `@/src/${dir}/*/**`, `!@/src/${dir}/index`],
message: `Import the ${dir} feature only from its public surface \`@/src/${dir}\` (src/${dir}/index.ts). A deep import bypasses the feature boundary.`,
});
const publicFeatures = features.filter(
(feature) => feature.publicSurface !== false
);
const otherSurfaces = (feature) =>
features.filter((other) => other !== feature).map(publicSurface);
publicFeatures.filter((other) => other !== feature).map(publicSurface);

return [
{
Expand All @@ -38,7 +82,7 @@ const featureBoundaries = (features) => {
rules: {
'no-restricted-imports': [
'error',
{ patterns: features.map(publicSurface) },
{ patterns: publicFeatures.map(publicSurface) },
],
},
},
Expand All @@ -52,28 +96,37 @@ const featureBoundaries = (features) => {
},
})),
...features.map((feature) => ({
files: feature.pure.files,
files: feature.pure,
ignores: ['**/__tests__/**'],
rules: {
'no-restricted-imports': [
'error',
{
paths: ['pinia', 'vue'].map((name) => ({
name,
message: `The ${feature.dir} pure layer must stay framework-free: no ${name}.`,
})),
paths: [
...['pinia', 'vue'].map((name) => ({
name,
message: `The ${feature.dir} pure layer must stay framework-free: no ${name}.`,
})),
// The bare directory is the index, an upper module.
{
name: `@/src/${feature.dir}`,
message: upperLayerMessage(feature),
},
],
patterns: [
{
group: [
...feature.pure.upperModules.flatMap((mod) => [
`@/src/${feature.dir}/${mod}`,
`./${mod}`,
`../${mod}`,
]),
...ownFeatureExceptPure(feature),
'@/src/store/**',
'@/src/components/**',
'@/src/composables/**',
],
message: `The ${feature.dir} pure layer must not import stores, components, or upper feature modules — dependencies point downward only.`,
message: upperLayerMessage(feature),
},
{
// The list above can only judge the alias spelling.
regex: '^\\.\\.?(/|$)',
message: `The ${feature.dir} pure layer spells its imports from \`@/src/\`, so the pure-layer boundary can see them.`,
},
...otherSurfaces(feature),
],
Expand Down Expand Up @@ -168,6 +221,17 @@ export default tseslint.config(
'@typescript-eslint/no-unsafe-function-type': 'off',
},
},
// Node tooling (scripts/checks/, fixture servers). The block above matches
// only .js/.ts/.vue, so without this eslint reports `process`, `console` and
// `URL` as undefined in every .mjs file.
{
files: ['**/*.mjs'],
languageOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
globals: globals.node,
},
},
{
files: ['**/tests/pageobjects/**/*.ts'],
rules: {
Expand Down Expand Up @@ -246,33 +310,77 @@ export default tseslint.config(
},
},
...featureBoundaries([
{
dir: 'segmentation',
pure: [
'src/segmentation/geometry.ts',
'src/segmentation/color.ts',
'src/segmentation/model.ts',
'src/segmentation/segment.ts',
'src/segmentation/masks/storage.ts',
'src/segmentation/masks/overlap.ts',
'src/segmentation/masks/labelValue.ts',
'src/segmentation/editing/algorithms/fillHoles.ts',
'src/segmentation/editing/algorithms/fillHoles.worker.ts',
'src/segmentation/editing/algorithms/gaussianSmooth.worker.ts',
],
// Consumers import explicit modules; the pure-layer rule still applies.
publicSurface: false,
},
{
dir: 'processing',
pure: {
files: [
'src/processing/engine/**/*.{js,ts}',
'src/processing/types.ts',
'src/processing/config.ts',
],
upperModules: [
'store',
'applyResults',
'jobResultReview',
'index',
'components/**',
],
},
pure: [
'src/processing/engine/**/*.{js,ts}',
'src/processing/types.ts',
'src/processing/config.ts',
],
},
{
dir: 'referenceLines',
pure: {
files: [
'src/referenceLines/geometry.ts',
'src/referenceLines/crossings.ts',
],
upperModules: ['store', 'index', 'useReferenceLines', 'components/**'],
},
pure: [
'src/referenceLines/geometry.ts',
'src/referenceLines/crossings.ts',
],
},
]),
// Tests are excluded so this block never matches a file the `vi.mock` block
// above matches: flat config replaces a rule's options wholesale, so overlap
// would erase that rule rather than add to it.
{
files: ['src/**/*.{ts,vue}'],
ignores: ['src/**/__tests__/**', 'src/**/*.{spec,test}.{js,ts}'],
rules: {
'no-restricted-syntax': [
'warn',
{
selector:
"CallExpression[callee.name='computed'][typeArguments.params.length>0]",
message:
'Let computed() infer its type. An explicit generic goes stale and hides the inference errors it was meant to document.',
},
],
},
},
// Mirrors the limits scripts/checks/complexity.mjs enforces per commit. Warn
// level because the ratchet only fails a file whose debt grows, so existing
// files stay over these numbers without failing `npm run lint`.
{
files: ['src/**/*.{js,ts,vue}'],
ignores: [
'src/**/__tests__/**',
'src/**/*.{spec,test}.{js,ts}',
'src/**/*.d.ts',
'**/emscripten-build/**',
],
rules: {
complexity: ['warn', 10],
'max-depth': ['warn', 3],
'max-params': ['warn', 4],
'max-lines': [
'warn',
{ max: 600, skipBlankLines: true, skipComments: true },
],
},
},
eslintConfigPrettier
);
Loading
Loading