diff --git a/.changeset/tall-dragons-bathe.md b/.changeset/tall-dragons-bathe.md new file mode 100644 index 0000000..37e565b --- /dev/null +++ b/.changeset/tall-dragons-bathe.md @@ -0,0 +1,6 @@ +--- +'@feature-sliced/steiger-plugin': minor +'@steiger/types': minor +--- + +Add location information to the diagnostic and refactor the rules to supply it diff --git a/packages/steiger-plugin-fsd/src/_lib/group-slices.ts b/packages/steiger-plugin-fsd/src/_lib/group-slices.ts index e8c4a67..e6e7937 100644 --- a/packages/steiger-plugin-fsd/src/_lib/group-slices.ts +++ b/packages/steiger-plugin-fsd/src/_lib/group-slices.ts @@ -1,5 +1,12 @@ import { sep, join } from 'node:path' +/** + * Groups slice names that share the same parent path segments. + * + * @example + * groupSlices(['a/b/c', 'a/b/d', 'a/e', 'f', 'g']) + * // => { 'a/b': ['c', 'd'], a: ['e'], '': ['f', 'g'] } + */ export function groupSlices(sliceNames: Array): Record> { const groups: Record> = {} diff --git a/packages/steiger-plugin-fsd/src/_lib/prepare-test.ts b/packages/steiger-plugin-fsd/src/_lib/prepare-test.ts index bd7fa0a..cb84c08 100644 --- a/packages/steiger-plugin-fsd/src/_lib/prepare-test.ts +++ b/packages/steiger-plugin-fsd/src/_lib/prepare-test.ts @@ -37,7 +37,7 @@ export function parseIntoFsdRoot(fsMarkup: string): FsdRoot { } export function compareMessages(a: Diagnostic, b: Diagnostic): number { - return a.message.localeCompare(b.message) + return a.message.localeCompare(b.message) || a.location.path.localeCompare(b.location.path) } export function joinFromRoot(...segments: Array) { diff --git a/packages/steiger-plugin-fsd/src/ambiguous-slice-names/README.md b/packages/steiger-plugin-fsd/src/ambiguous-slice-names/README.md index e9d767a..5ca7055 100644 --- a/packages/steiger-plugin-fsd/src/ambiguous-slice-names/README.md +++ b/packages/steiger-plugin-fsd/src/ambiguous-slice-names/README.md @@ -44,6 +44,29 @@ Examples of project structures that fail this rule: πŸ“„ index.ts ``` +``` +πŸ“‚ shared + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ i18n // ❗️ (1) + πŸ“„ index.ts + πŸ“‚ store // ❗️ (2) + πŸ“„ index.ts +πŸ“‚ features + πŸ“‚ i18n // ❌ (1) + πŸ“‚ grouped + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ test + πŸ“‚ store // ❌ (2) + πŸ“‚ ui + πŸ“„ index.ts +πŸ“‚ pages + πŸ“‚ home + πŸ“‚ ui + πŸ“„ index.ts +``` + ## Rationale When there is a segment in shared with the same name as a slice, it becomes ambiguous where new code should be written, and also obscures the search for code. diff --git a/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.spec.ts b/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.spec.ts index 3e3ba91..51b4879 100644 --- a/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.spec.ts @@ -1,7 +1,8 @@ +import { join } from 'node:path' import { expect, it } from 'vitest' import ambiguousSliceNames from './index.js' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' it('reports no errors on a project without slice names that match some segment name in Shared', () => { const root = parseIntoFsdRoot(` @@ -48,6 +49,46 @@ it('reports errors on a project with slice names that match some segment name in const diagnostics = ambiguousSliceNames.check(root).diagnostics expect(diagnostics).toEqual([ - { message: 'Slice name "i18n" on layer "features" is ambiguous with a segment name in Shared' }, + { + message: 'Slice "i18n" could be confused with a segment from Shared with the same name', + location: { path: joinFromRoot('features', 'i18n') }, + }, + ]) +}) + +it('works for slice groups and grouped slices', () => { + const root = parseIntoFsdRoot(` + πŸ“‚ shared + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ i18n + πŸ“„ index.ts + πŸ“‚ store + πŸ“„ index.ts + πŸ“‚ features + πŸ“‚ i18n + πŸ“‚ grouped + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ test + πŸ“‚ store + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ pages + πŸ“‚ home + πŸ“‚ ui + πŸ“„ index.ts + `) + + const diagnostics = ambiguousSliceNames.check(root).diagnostics + expect(diagnostics).toEqual([ + { + message: 'Slice group "i18n" could be confused with a segment "i18n" from Shared', + location: { path: joinFromRoot('features', 'i18n') }, + }, + { + message: `Slice "${join('test', 'store')}" could be confused with a segment "store" from Shared`, + location: { path: joinFromRoot('features', 'test', 'store') }, + }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.ts b/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.ts index 1f7add8..15587ff 100644 --- a/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.ts +++ b/packages/steiger-plugin-fsd/src/ambiguous-slice-names/index.ts @@ -1,6 +1,6 @@ -import { sep } from 'node:path' -import { getAllSlices, getLayers, getSegments } from '@feature-sliced/filesystem' -import type { Diagnostic, Rule } from '@steiger/types' +import { basename, sep } from 'node:path' +import { getAllSlices, getLayers, getSegments, type LayerName } from '@feature-sliced/filesystem' +import type { Diagnostic, Folder, Rule } from '@steiger/types' /** Forbid slice names that match some segment’s name in shared (e.g., theme, i18n) */ const ambiguousSliceNames = { @@ -20,14 +20,46 @@ const ambiguousSliceNames = { for (const [sliceName, slice] of Object.entries(getAllSlices(root))) { const pathSegments = sliceName.split(sep) const matchingSegment = segmentNamesInShared.find((segmentName) => pathSegments.includes(segmentName)) - if (matchingSegment !== undefined) { - if (matchingSegment === sliceName) { - diagnostics.push({ - message: `Slice name "${sliceName}" on layer "${slice.layerName}" is ambiguous with a segment name in Shared`, - }) - } else { + + if (matchingSegment === undefined) { + continue + } + + if (matchingSegment === sliceName) { + diagnostics.push({ + message: `Slice "${sliceName}" could be confused with a segment from Shared with the same name`, + location: { path: slice.path }, + }) + continue + } + + const segmentIndex = pathSegments.indexOf(matchingSegment) + const isGroup = segmentIndex < pathSegments.length - 1 + if (!isGroup) { + diagnostics.push({ + message: `Slice "${sliceName}" could be confused with a segment "${matchingSegment}" from Shared`, + location: { path: slice.path }, + }) + } else { + const layer = layers[slice.layerName as LayerName] + if (layer === undefined) { + continue + } + + const pathSegmentsToGroup = pathSegments.slice(0, segmentIndex + 1) + + const group = pathSegmentsToGroup.reduce( + (folder, segmentName) => + folder?.children.find((child) => child.type === 'folder' && basename(child.path) === segmentName) as + | Folder + | undefined, + layer, + ) + + if (group !== undefined) { diagnostics.push({ - message: `Slice name "${sliceName}" on layer "${slice.layerName}" is ambiguous with a segment name "${matchingSegment}" in Shared`, + message: `Slice group "${pathSegmentsToGroup.join(sep)}" could be confused with a segment "${matchingSegment}" from Shared`, + location: { path: group.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/excessive-slicing/index.spec.ts b/packages/steiger-plugin-fsd/src/excessive-slicing/index.spec.ts index 17556bd..54ba725 100644 --- a/packages/steiger-plugin-fsd/src/excessive-slicing/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/excessive-slicing/index.spec.ts @@ -1,6 +1,6 @@ import { expect, it } from 'vitest' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' import excessiveSlicing from './index.js' it('reports no errors on projects with moderate slicing', () => { @@ -112,8 +112,109 @@ it('reports errors on a project with an excessive amount of features', () => { expect(diagnostics).toEqual([ { + location: { path: joinFromRoot('features') }, message: 'Layer "features" has 23 ungrouped slices, which is above the recommended threshold of 20. Consider grouping them or moving the code inside to the layer where it\'s used.', }, ]) }) + +it('works with slice groups', () => { + const root = parseIntoFsdRoot(` + πŸ“‚ entities + πŸ“‚ users + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ posts + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ shared + πŸ“‚ ui + πŸ“„ index.ts + πŸ“„ Button.tsx + πŸ“‚ features + πŸ“‚ legit-feature + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ junk + πŸ“‚ comments + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ posts + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ users + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ cars + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ alligators + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ whales + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ giraffes + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ buses + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ trains + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ planes + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ boats + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ submarines + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ helicopters + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ rockets + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ satellites + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ space-stations + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ planets + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ galaxies + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ universes + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ multiverses + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ metaverses + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ ai + πŸ“‚ ui + πŸ“„ index.ts + πŸ“‚ bitcoin + πŸ“‚ ui + πŸ“„ index.ts + `) + + const { diagnostics } = excessiveSlicing.check(root) + + expect(diagnostics).toEqual([ + { + location: { path: joinFromRoot('features', 'junk') }, + message: + 'Slice group "junk" has 23 slices, which is above the recommended threshold of 20. Consider grouping them or moving the code inside to the layer where it\'s used.', + }, + ]) +}) diff --git a/packages/steiger-plugin-fsd/src/excessive-slicing/index.ts b/packages/steiger-plugin-fsd/src/excessive-slicing/index.ts index 7a01537..6d8ab26 100644 --- a/packages/steiger-plugin-fsd/src/excessive-slicing/index.ts +++ b/packages/steiger-plugin-fsd/src/excessive-slicing/index.ts @@ -1,5 +1,7 @@ +import { join } from 'node:path' import { getLayers, getSlices, isSliced } from '@feature-sliced/filesystem' import type { Diagnostic, Rule } from '@steiger/types' + import { groupSlices } from '../_lib/group-slices.js' const THRESHOLDS = { @@ -16,7 +18,7 @@ const excessiveSlicing = { const diagnostics: Array = [] for (const [layerName, layer] of Object.entries(getLayers(root))) { - if (!isSliced(layer)) { + if (!isSliced(layer) || !(layerName in THRESHOLDS)) { continue } @@ -24,9 +26,19 @@ const excessiveSlicing = { const threshold = THRESHOLDS[layerName as keyof typeof THRESHOLDS] for (const [group, slices] of Object.entries(sliceGroups)) { - if (slices.length > threshold) { + if (slices.length <= threshold) { + continue + } + + if (group === '') { + diagnostics.push({ + message: `Layer "${layerName}" has ${slices.length} ungrouped slices, which is above the recommended threshold of ${threshold}. Consider grouping them or moving the code inside to the layer where it's used.`, + location: { path: layer.path }, + }) + } else { diagnostics.push({ - message: `Layer "${layerName}" has ${slices.length} ${group === '' ? 'ungrouped slices' : `slices in group "${group}"`}, which is above the recommended threshold of ${threshold}. Consider grouping them or moving the code inside to the layer where it's used.`, + message: `Slice group "${group}" has ${slices.length} slices, which is above the recommended threshold of ${threshold}. Consider grouping them or moving the code inside to the layer where it's used.`, + location: { path: join(layer.path, group) }, }) } } diff --git a/packages/steiger-plugin-fsd/src/forbidden-imports/index.spec.ts b/packages/steiger-plugin-fsd/src/forbidden-imports/index.spec.ts index 77d9913..9f10e17 100644 --- a/packages/steiger-plugin-fsd/src/forbidden-imports/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/forbidden-imports/index.spec.ts @@ -1,7 +1,6 @@ import { expect, it, vi } from 'vitest' -import { join } from 'node:path' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' import forbiddenImports from './index.js' vi.mock('tsconfck', async (importOriginal) => { @@ -82,7 +81,8 @@ it('reports errors on a project with cross-imports in entities', async () => { expect((await forbiddenImports.check(root)).diagnostics).toEqual([ { - message: `Forbidden cross-import on layer "entities" from "${join('product', 'ui', 'ProductCard.tsx')}" to slice "user".`, + message: `Forbidden cross-import from slice "user".`, + location: { path: joinFromRoot('entities', 'product', 'ui', 'ProductCard.tsx') }, }, ]) }) @@ -111,7 +111,8 @@ it('reports errors on a project where a feature imports from a page', async () = expect((await forbiddenImports.check(root)).diagnostics).toEqual([ { - message: `Forbidden import from "${join('features', 'comments', 'ui', 'CommentCard.tsx')}" to higher layer "pages".`, + message: `Forbidden import from higher layer "pages".`, + location: { path: joinFromRoot('features', 'comments', 'ui', 'CommentCard.tsx') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/forbidden-imports/index.ts b/packages/steiger-plugin-fsd/src/forbidden-imports/index.ts index 6c8853b..165648a 100644 --- a/packages/steiger-plugin-fsd/src/forbidden-imports/index.ts +++ b/packages/steiger-plugin-fsd/src/forbidden-imports/index.ts @@ -1,5 +1,4 @@ import * as fs from 'node:fs' -import { relative, join } from 'node:path' import { layerSequence, resolveImport } from '@feature-sliced/filesystem' import precinct from 'precinct' const { paperwork } = precinct @@ -38,14 +37,15 @@ const forbiddenImports = { sourceFile.layerName === dependencyLocation.layerName && sourceFile.sliceName !== dependencyLocation.sliceName ) { - const relativePath = relative(join(root.path, sourceFile.layerName), sourceFile.file.path) if (dependencyLocation.sliceName === null) { diagnostics.push({ - message: `Forbidden cross-import on layer "${dependencyLocation.layerName}" from "${relativePath}" to segment "${dependencyLocation.segmentName}".`, + message: `Forbidden cross-import from segment "${dependencyLocation.segmentName}".`, + location: { path: sourceFile.file.path }, }) } else { diagnostics.push({ - message: `Forbidden cross-import on layer "${dependencyLocation.layerName}" from "${relativePath}" to slice "${dependencyLocation.sliceName}".`, + message: `Forbidden cross-import from slice "${dependencyLocation.sliceName}".`, + location: { path: sourceFile.file.path }, }) } } else { @@ -54,7 +54,8 @@ const forbiddenImports = { if (thisLayerIndex < dependencyLayerIndex) { diagnostics.push({ - message: `Forbidden import from "${relative(root.path, sourceFile.file.path)}" to higher layer "${dependencyLocation.layerName}".`, + message: `Forbidden import from higher layer "${dependencyLocation.layerName}".`, + location: { path: sourceFile.file.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/import-locality/index.spec.ts b/packages/steiger-plugin-fsd/src/import-locality/index.spec.ts index b179ba6..2025b8d 100644 --- a/packages/steiger-plugin-fsd/src/import-locality/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/import-locality/index.spec.ts @@ -1,7 +1,6 @@ -import { join } from 'node:path' import { expect, it, vi } from 'vitest' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' import importLocality from './index.js' vi.mock('tsconfck', async (importOriginal) => { @@ -80,7 +79,8 @@ it('reports errors on a project with absolute imports within a slice', async () expect((await importLocality.check(root)).diagnostics).toEqual([ { - message: `Import on "${join('entities', 'user')}" from "${join('ui', 'UserAvatar.tsx')}" to "${join('ui', 'Name.tsx')}" should be relative.`, + message: `Import from "@/entities/user/ui/Name" should be relative.`, + location: { path: joinFromRoot('entities', 'user', 'ui', 'UserAvatar.tsx') }, }, ]) }) @@ -109,7 +109,8 @@ it('reports errors on a project with absolute imports from the index within a sl expect((await importLocality.check(root)).diagnostics).toEqual([ { - message: `Import on "${join('entities', 'user')}" from "${join('ui', 'Status.tsx')}" to "index.ts" should be relative.`, + message: `Import from "@/entities/user" should be relative.`, + location: { path: joinFromRoot('entities', 'user', 'ui', 'Status.tsx') }, }, ]) }) @@ -142,7 +143,8 @@ it('reports errors on a project with relative imports between slices', async () expect((await importLocality.check(root)).diagnostics).toEqual([ { - message: `Import from "${join('features', 'comments', 'ui', 'CommentCard.tsx')}" to "${join('entities', 'user', 'index.ts')} should not be relative.`, + message: `Import from "../../../entities/user" should not be relative.`, + location: { path: joinFromRoot('features', 'comments', 'ui', 'CommentCard.tsx') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/import-locality/index.ts b/packages/steiger-plugin-fsd/src/import-locality/index.ts index 874a81b..a962d36 100644 --- a/packages/steiger-plugin-fsd/src/import-locality/index.ts +++ b/packages/steiger-plugin-fsd/src/import-locality/index.ts @@ -1,5 +1,4 @@ import * as fs from 'node:fs' -import { relative, join } from 'node:path' import { resolveImport } from '@feature-sliced/filesystem' import precinct from 'precinct' const { paperwork } = precinct @@ -39,24 +38,14 @@ const importLocality = { sourceFile.layerName === dependencyLocation.layerName && sourceFile.sliceName === dependencyLocation.sliceName if (isRelative && !isWithinSameSlice) { - const sourceRelativePath = relative(root.path, sourceFile.file.path) - const dependencyRelativePath = relative(root.path, dependencyLocation.file.path) diagnostics.push({ - message: `Import from "${sourceRelativePath}" to "${dependencyRelativePath} should not be relative.`, + message: `Import from "${dependency}" should not be relative.`, + location: { path: sourceFile.file.path }, }) } else if (!isRelative && isWithinSameSlice) { - const sourceRelativePath = relative( - join(...[root.path, sourceFile.layerName, sourceFile.sliceName].filter(Boolean)), - sourceFile.file.path, - ) - const dependencyRelativePath = relative( - join(...[root.path, dependencyLocation.layerName, dependencyLocation.sliceName].filter(Boolean)), - dependencyLocation.file.path, - ) - const layerAndSlice = join(...[dependencyLocation.layerName, dependencyLocation.sliceName].filter(Boolean)) - diagnostics.push({ - message: `Import on "${layerAndSlice}" from "${sourceRelativePath}" to "${dependencyRelativePath}" should be relative.`, + message: `Import from "${dependency}" should be relative.`, + location: { path: sourceFile.file.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/inconsistent-naming/index.spec.ts b/packages/steiger-plugin-fsd/src/inconsistent-naming/index.spec.ts index 9fcf486..3e7b606 100644 --- a/packages/steiger-plugin-fsd/src/inconsistent-naming/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/inconsistent-naming/index.spec.ts @@ -31,7 +31,7 @@ it('reports an error on slice names that are not pluralized consistently', () => const diagnostics = inconsistentNaming.check(root).diagnostics.sort(compareMessages) expect(diagnostics).toEqual([ { - message: 'Inconsistent pluralization on layer "entities". Prefer all plural names', + message: 'Inconsistent pluralization of slice names. Prefer all plural names', fixes: [ { type: 'rename', @@ -39,6 +39,7 @@ it('reports an error on slice names that are not pluralized consistently', () => newName: 'users', }, ], + location: { path: joinFromRoot('entities') }, }, ]) }) @@ -60,7 +61,7 @@ it('prefers the singular form when there are more singular slices', () => { const diagnostics = inconsistentNaming.check(root).diagnostics.sort(compareMessages) expect(diagnostics).toEqual([ { - message: 'Inconsistent pluralization on layer "entities". Prefer all singular names', + message: 'Inconsistent pluralization of slice names. Prefer all singular names', fixes: [ { type: 'rename', @@ -68,6 +69,7 @@ it('prefers the singular form when there are more singular slices', () => { newName: 'comment', }, ], + location: { path: joinFromRoot('entities') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/inconsistent-naming/index.ts b/packages/steiger-plugin-fsd/src/inconsistent-naming/index.ts index 3e48f0d..1397611 100644 --- a/packages/steiger-plugin-fsd/src/inconsistent-naming/index.ts +++ b/packages/steiger-plugin-fsd/src/inconsistent-naming/index.ts @@ -24,11 +24,7 @@ const inconsistentNaming = { const [pluralNames, singularNames] = partition(group, isPlural) if (pluralNames.length > 0 && singularNames.length > 0) { - let message = `Inconsistent pluralization on layer "entities"` - - if (groupPrefix.length > 0) { - message += ` for slice group "${groupPrefix}"` - } + const message = 'Inconsistent pluralization of slice names' if (pluralNames.length >= singularNames.length) { diagnostics.push({ @@ -38,6 +34,7 @@ const inconsistentNaming = { path: join(entities.path, groupPrefix, name), newName: plural(name), })), + location: { path: join(entities.path, groupPrefix) }, }) } else { diagnostics.push({ @@ -47,6 +44,7 @@ const inconsistentNaming = { path: join(entities.path, groupPrefix, name), newName: singular(name), })), + location: { path: join(entities.path, groupPrefix) }, }) } } diff --git a/packages/steiger-plugin-fsd/src/insignificant-slice/index.spec.ts b/packages/steiger-plugin-fsd/src/insignificant-slice/index.spec.ts index 64af733..2033147 100644 --- a/packages/steiger-plugin-fsd/src/insignificant-slice/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/insignificant-slice/index.spec.ts @@ -1,7 +1,7 @@ import { expect, it, vi } from 'vitest' import { join } from 'node:path' -import { compareMessages, parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { compareMessages, joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' import insignificantSlice from './index.js' vi.mock('tsconfck', async (importOriginal) => { @@ -123,10 +123,12 @@ it('reports errors on a project with insignificant slices', async () => { expect((await insignificantSlice.check(root)).diagnostics.sort(compareMessages)).toEqual([ { - message: `Slice "${join('entities', 'product')}" has no references. Consider removing it.`, + message: `This slice has no references. Consider removing it.`, + location: { path: joinFromRoot('entities', 'product') }, }, { - message: `Slice "${join('entities', 'user')}" has only one reference in slice "${join('pages', 'editor')}". Consider merging them.`, + message: `This slice has only one reference in slice "${join('pages', 'editor')}". Consider merging them.`, + location: { path: joinFromRoot('entities', 'user') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/insignificant-slice/index.ts b/packages/steiger-plugin-fsd/src/insignificant-slice/index.ts index 365e40c..237f52b 100644 --- a/packages/steiger-plugin-fsd/src/insignificant-slice/index.ts +++ b/packages/steiger-plugin-fsd/src/insignificant-slice/index.ts @@ -1,5 +1,5 @@ import * as fs from 'node:fs' -import { sep } from 'node:path' +import { sep, join } from 'node:path' import { parse as parseNearestTsConfig } from 'tsconfck' import { isSliced, resolveImport, unslicedLayers, type LayerName } from '@feature-sliced/filesystem' import type { Folder, Diagnostic, Rule } from '@steiger/types' @@ -25,16 +25,19 @@ const insignificantSlice = { const referenceLocationKey = [...targetLocationKeys][0] if (unslicedLayers.includes(referenceLocationKey)) { diagnostics.push({ - message: `Slice "${sourceLocationKey}" has only one reference on layer "${referenceLocationKey}". Consider merging them.`, + message: `This slice has only one reference on layer "${referenceLocationKey}". Consider moving this code to "${referenceLocationKey}".`, + location: { path: join(root.path, sourceLocationKey) }, }) } else { diagnostics.push({ - message: `Slice "${sourceLocationKey}" has only one reference in slice "${[...targetLocationKeys][0]}". Consider merging them.`, + message: `This slice has only one reference in slice "${referenceLocationKey}". Consider merging them.`, + location: { path: join(root.path, sourceLocationKey) }, }) } } else if (targetLocationKeys.size === 0) { diagnostics.push({ - message: `Slice "${sourceLocationKey}" has no references. Consider removing it.`, + message: `This slice has no references. Consider removing it.`, + location: { path: join(root.path, sourceLocationKey) }, }) } } diff --git a/packages/steiger-plugin-fsd/src/no-file-segments/index.spec.ts b/packages/steiger-plugin-fsd/src/no-file-segments/index.spec.ts index 2d1df46..5e186a2 100644 --- a/packages/steiger-plugin-fsd/src/no-file-segments/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-file-segments/index.spec.ts @@ -1,7 +1,6 @@ import { expect, it } from 'vitest' -import { join } from 'node:path' -import { compareMessages, parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { compareMessages, joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' import noFileSegments from './index.js' it('reports no errors on a project with only folder segments', async () => { @@ -47,10 +46,12 @@ it('reports no errors on a project with folder segments on sliced layers', async expect(noFileSegments.check(root).diagnostics).toEqual([ { - message: `In "${join('features', 'comments')}", "ui.tsx" is a file segment. Prefer folder segments.`, + message: 'This segment is a file. Prefer folder segments.', + location: { path: joinFromRoot('features', 'comments', 'ui.tsx') }, }, { - message: `In "${join('pages', 'editor')}", "ui.tsx" is a file segment. Prefer folder segments.`, + message: 'This segment is a file. Prefer folder segments.', + location: { path: joinFromRoot('pages', 'editor', 'ui.tsx') }, }, ]) }) @@ -92,7 +93,8 @@ it('reports errors on a project with folder segments in Shared', async () => { expect(noFileSegments.check(root).diagnostics.sort(compareMessages)).toEqual([ { - message: `On layer "shared", "routes.ts" is a file segment. Prefer folder segments.`, + message: 'This segment is a file. Prefer folder segments.', + location: { path: joinFromRoot('shared', 'routes.ts') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/no-file-segments/index.ts b/packages/steiger-plugin-fsd/src/no-file-segments/index.ts index a7f4c47..7585e9e 100644 --- a/packages/steiger-plugin-fsd/src/no-file-segments/index.ts +++ b/packages/steiger-plugin-fsd/src/no-file-segments/index.ts @@ -1,4 +1,4 @@ -import { basename, relative } from 'node:path' +import { basename } from 'node:path' import { getLayers, getSlices, isSliced } from '@feature-sliced/filesystem' import type { Diagnostic, Rule } from '@steiger/types' @@ -7,12 +7,13 @@ const noFileSegments = { check(root) { const diagnostics: Array = [] - for (const [layerName, layer] of Object.entries(getLayers(root))) { + for (const layer of Object.values(getLayers(root))) { if (!isSliced(layer)) { for (const child of layer.children) { if (child.type === 'file') { diagnostics.push({ - message: `On layer "${layerName}", "${basename(child.path)}" is a file segment. Prefer folder segments.`, + message: 'This segment is a file. Prefer folder segments.', + location: { path: child.path }, }) } } @@ -21,7 +22,8 @@ const noFileSegments = { for (const child of slice.children) { if (child.type === 'file' && withoutExtension(basename(child.path)) !== 'index') { diagnostics.push({ - message: `In "${relative(root.path, slice.path)}", "${basename(child.path)}" is a file segment. Prefer folder segments.`, + message: 'This segment is a file. Prefer folder segments.', + location: { path: child.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/no-layer-public-api/index.spec.ts b/packages/steiger-plugin-fsd/src/no-layer-public-api/index.spec.ts index b720099..548a2e9 100644 --- a/packages/steiger-plugin-fsd/src/no-layer-public-api/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-layer-public-api/index.spec.ts @@ -1,7 +1,7 @@ import { expect, it } from 'vitest' import noLayerPublicApi from './index.js' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' it('reports no errors on a project without index files on layer level', () => { const root = parseIntoFsdRoot(` @@ -46,7 +46,7 @@ it('reports errors on a project with index files on layer level', () => { const diagnostics = noLayerPublicApi.check(root).diagnostics expect(diagnostics).toEqual([ - { message: 'Layer "shared" should not have an index file' }, - { message: 'Layer "pages" should not have an index file' }, + { message: 'Layer "shared" should not have an index file', location: { path: joinFromRoot('shared', 'index.ts') } }, + { message: 'Layer "pages" should not have an index file', location: { path: joinFromRoot('pages', 'index.ts') } }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/no-layer-public-api/index.ts b/packages/steiger-plugin-fsd/src/no-layer-public-api/index.ts index 5c24ea5..3246efe 100644 --- a/packages/steiger-plugin-fsd/src/no-layer-public-api/index.ts +++ b/packages/steiger-plugin-fsd/src/no-layer-public-api/index.ts @@ -10,7 +10,10 @@ const noLayerPublicApi = { for (const [layerName, layer] of Object.entries(getLayers(root))) { const index = getIndex(layer) if (index !== undefined) { - diagnostics.push({ message: `Layer "${layerName}" should not have an index file` }) + diagnostics.push({ + message: `Layer "${layerName}" should not have an index file`, + location: { path: index.path }, + }) } } diff --git a/packages/steiger-plugin-fsd/src/no-processes/index.spec.ts b/packages/steiger-plugin-fsd/src/no-processes/index.spec.ts index d701e5d..169f940 100644 --- a/packages/steiger-plugin-fsd/src/no-processes/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-processes/index.spec.ts @@ -1,7 +1,7 @@ import { expect, it } from 'vitest' import noProcesses from './index.js' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' it('reports no errors on a project without the Processes layer', () => { const root = parseIntoFsdRoot(` @@ -47,5 +47,7 @@ it('reports errors on a project with the Processes layer', () => { `) const diagnostics = noProcesses.check(root).diagnostics - expect(diagnostics).toEqual([{ message: 'Layer "processes" is deprecated, avoid using it' }]) + expect(diagnostics).toEqual([ + { message: 'Layer "processes" is deprecated, avoid using it', location: { path: joinFromRoot('processes') } }, + ]) }) diff --git a/packages/steiger-plugin-fsd/src/no-processes/index.ts b/packages/steiger-plugin-fsd/src/no-processes/index.ts index b93c467..c69d835 100644 --- a/packages/steiger-plugin-fsd/src/no-processes/index.ts +++ b/packages/steiger-plugin-fsd/src/no-processes/index.ts @@ -13,6 +13,7 @@ const noProcesses = { if (processesLayer !== undefined) { diagnostics.push({ message: 'Layer "processes" is deprecated, avoid using it', + location: { path: processesLayer.path }, }) } diff --git a/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.spec.ts b/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.spec.ts index 78e7335..54b5b65 100644 --- a/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.spec.ts @@ -1,7 +1,6 @@ import { expect, it, vi } from 'vitest' -import { join } from 'node:path' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' import noPublicApiSidestep from './index.js' vi.mock('tsconfck', async (importOriginal) => { @@ -87,7 +86,8 @@ it('reports errors on a project with a public API sidestep on entities', async ( expect((await noPublicApiSidestep.check(root)).diagnostics).toEqual([ { - message: `Forbidden sidestep of public API when importing "${join('entities', 'product', 'ui', 'ProductCard.tsx')}" from "${join('pages', 'editor', 'ui', 'Editor.tsx')}".`, + message: `Forbidden sidestep of public API when importing from "@/entities/product/ui/ProductCard.tsx".`, + location: { path: joinFromRoot('pages', 'editor', 'ui', 'Editor.tsx') }, }, ]) }) @@ -110,7 +110,8 @@ it('reports errors on a project with a public API sidestep on shared', async () expect((await noPublicApiSidestep.check(root)).diagnostics).toEqual([ { - message: `Forbidden sidestep of public API when importing "${join('shared', 'ui', 'Button.tsx')}" from "${join('pages', 'editor', 'ui', 'SubmitButton.tsx')}".`, + message: `Forbidden sidestep of public API when importing from "@/shared/ui/Button".`, + location: { path: joinFromRoot('pages', 'editor', 'ui', 'SubmitButton.tsx') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.ts b/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.ts index a29a329..f2d2fad 100644 --- a/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.ts +++ b/packages/steiger-plugin-fsd/src/no-public-api-sidestep/index.ts @@ -1,5 +1,4 @@ import * as fs from 'node:fs' -import { relative } from 'node:path' import precinct from 'precinct' const { paperwork } = precinct import { parse as parseNearestTsConfig } from 'tsconfck' @@ -46,7 +45,8 @@ const noPublicApiSidestep = { if (isSliced(dependencyLocation.layerName)) { if (dependencyLocation.segmentName !== null) { diagnostics.push({ - message: `Forbidden sidestep of public API when importing "${relative(root.path, resolvedDependency)}" from "${relative(root.path, sourceFile.file.path)}".`, + message: `Forbidden sidestep of public API when importing from "${dependency}".`, + location: { path: sourceFile.file.path }, }) } } else if (dependencyLocation.segmentName !== null) { @@ -63,7 +63,8 @@ const noPublicApiSidestep = { const index = getIndex(segment) if (resolvedDependency !== index?.path) { diagnostics.push({ - message: `Forbidden sidestep of public API when importing "${relative(root.path, resolvedDependency)}" from "${relative(root.path, sourceFile.file.path)}".`, + message: `Forbidden sidestep of public API when importing from "${dependency}".`, + location: { path: sourceFile.file.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.spec.ts b/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.spec.ts index 776d7fd..85d2a8d 100644 --- a/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.spec.ts @@ -1,8 +1,7 @@ import { expect, it } from 'vitest' -import { join } from 'node:path' import noReservedFolderNames from './index.js' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' it('reports no errors on a project without subfolders in segments that use reserved names', () => { const root = parseIntoFsdRoot(` @@ -45,6 +44,10 @@ it('reports errors on a project with subfolders in segments that use reserved na const diagnostics = noReservedFolderNames.check(root).diagnostics expect(diagnostics).toEqual([ - { message: `Folder name "lib" in "${join('shared', 'ui')}" is reserved for segment names` }, + { + message: + 'Having a folder with the name "lib" inside a segment could be confusing because that name is commonly used for segments. Consider renaming it.', + location: { path: joinFromRoot('shared', 'ui', 'lib') }, + }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.ts b/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.ts index a2a8803..1ff01ca 100644 --- a/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.ts +++ b/packages/steiger-plugin-fsd/src/no-reserved-folder-names/index.ts @@ -1,4 +1,4 @@ -import { basename, join, relative } from 'node:path' +import { basename } from 'node:path' import { getAllSegments, conventionalSegmentNames } from '@feature-sliced/filesystem' import type { Diagnostic, Rule } from '@steiger/types' @@ -10,7 +10,7 @@ const noReservedFolderNames = { check(root) { const diagnostics: Array = [] - for (const { segment, layerName, sliceName, segmentName } of getAllSegments(root)) { + for (const { segment } of getAllSegments(root)) { if (segment.type === 'file') { continue } @@ -24,9 +24,9 @@ const noReservedFolderNames = { child, (entry) => entry.type === 'folder' && conventionalSegmentNames.includes(basename(entry.path)), )) { - const semanticLocation = join(...[layerName, sliceName, segmentName].filter(Boolean)) diagnostics.push({ - message: `Folder name "${relative(segment.path, violatingFolder.path)}" in "${semanticLocation}" is reserved for segment names`, + message: `Having a folder with the name "${basename(violatingFolder.path)}" inside a segment could be confusing because that name is commonly used for segments. Consider renaming it.`, + location: { path: violatingFolder.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.spec.ts b/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.spec.ts index 306d640..db8404c 100644 --- a/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.spec.ts @@ -1,8 +1,7 @@ import { expect, it } from 'vitest' -import { join } from 'node:path' import noSegmentlessSlices from './index.js' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' it('reports no errors on a project where every slice has at least one segment', () => { const root = parseIntoFsdRoot(` @@ -53,7 +52,13 @@ it('reports errors on a project where some slices have no segments', () => { const diagnostics = noSegmentlessSlices.check(root).diagnostics expect(diagnostics).toEqual([ - { message: 'Slice "user" on layer "entities" has no segments' }, - { message: `Slice "${join('settings', 'profile')}" on layer "pages" has no segments` }, + { + message: 'This slice has no segments. Consider dividing the code inside into segments.', + location: { path: joinFromRoot('entities', 'user') }, + }, + { + message: 'This slice has no segments. Consider dividing the code inside into segments.', + location: { path: joinFromRoot('pages', 'settings', 'profile') }, + }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.ts b/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.ts index 56d5cad..832f1a2 100644 --- a/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.ts +++ b/packages/steiger-plugin-fsd/src/no-segmentless-slices/index.ts @@ -1,4 +1,3 @@ -import { relative } from 'node:path' import { getLayers, isSlice, isSliced } from '@feature-sliced/filesystem' import type { Folder, Diagnostic, Rule } from '@steiger/types' @@ -7,7 +6,7 @@ const noSegmentlessSlices = { check(root) { const diagnostics: Array = [] - for (const [layerName, layer] of Object.entries(getLayers(root))) { + for (const layer of Object.values(getLayers(root))) { if (!isSliced(layer)) { continue } @@ -26,7 +25,8 @@ const noSegmentlessSlices = { // The slice detection algorithm relies on the presence of a conventional segment if (!isSlice(sliceCandidate)) { diagnostics.push({ - message: `Slice "${relative(layer.path, sliceCandidate.path)}" on layer "${layerName}" has no segments`, + message: 'This slice has no segments. Consider dividing the code inside into segments.', + location: { path: sliceCandidate.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/public-api/index.spec.ts b/packages/steiger-plugin-fsd/src/public-api/index.spec.ts index 981e72c..2004f91 100644 --- a/packages/steiger-plugin-fsd/src/public-api/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/public-api/index.spec.ts @@ -50,7 +50,7 @@ it('reports errors on slices that are missing a public API', () => { const diagnostics = publicApi.check(root, { sourceFileExtension: 'ts' }).diagnostics.sort(compareMessages) expect(diagnostics).toEqual([ { - message: 'On the "entities" layer, slice "posts" is missing a public API.', + message: 'This slice is missing a public API.', fixes: [ { type: 'create-file', @@ -58,9 +58,10 @@ it('reports errors on slices that are missing a public API', () => { content: '', }, ], + location: { path: joinFromRoot('entities', 'posts') }, }, { - message: 'On the "pages" layer, slice "editor" is missing a public API.', + message: 'This slice is missing a public API.', fixes: [ { type: 'create-file', @@ -68,6 +69,7 @@ it('reports errors on slices that are missing a public API', () => { content: '', }, ], + location: { path: joinFromRoot('pages', 'editor') }, }, ]) }) @@ -99,7 +101,7 @@ it('reports errors on segments that are missing a public API', () => { const diagnostics = publicApi.check(root, { sourceFileExtension: 'ts' }).diagnostics.sort(compareMessages) expect(diagnostics).toEqual([ { - message: 'On the "shared" layer, segment "ui" is missing a public API.', + message: 'This segment is missing a public API.', fixes: [ { type: 'create-file', @@ -107,6 +109,7 @@ it('reports errors on segments that are missing a public API', () => { content: '', }, ], + location: { path: joinFromRoot('shared', 'ui') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/public-api/index.ts b/packages/steiger-plugin-fsd/src/public-api/index.ts index ae94afc..09b50ec 100644 --- a/packages/steiger-plugin-fsd/src/public-api/index.ts +++ b/packages/steiger-plugin-fsd/src/public-api/index.ts @@ -14,10 +14,10 @@ const publicApi = { // The app layer is the top-level layer, there's no need for public API. continue } - for (const [segmentName, segment] of Object.entries(getSegments(layer))) { + for (const segment of Object.values(getSegments(layer))) { if (getIndex(segment) === undefined) { diagnostics.push({ - message: `On the "${layerName}" layer, segment "${segmentName}" is missing a public API.`, + message: 'This segment is missing a public API.', fixes: [ { type: 'create-file', @@ -26,14 +26,15 @@ const publicApi = { content: '', }, ], + location: { path: segment.path }, }) } } } else { - for (const [sliceName, slice] of Object.entries(getSlices(layer))) { + for (const slice of Object.values(getSlices(layer))) { if (getIndex(slice) === undefined) { diagnostics.push({ - message: `On the "${layerName}" layer, slice "${sliceName}" is missing a public API.`, + message: 'This slice is missing a public API.', fixes: [ { type: 'create-file', @@ -42,6 +43,7 @@ const publicApi = { content: '', }, ], + location: { path: slice.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/repetitive-naming/index.spec.ts b/packages/steiger-plugin-fsd/src/repetitive-naming/index.spec.ts index f8f0e63..817dd9e 100644 --- a/packages/steiger-plugin-fsd/src/repetitive-naming/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/repetitive-naming/index.spec.ts @@ -1,7 +1,7 @@ import { expect, it } from 'vitest' import repetitiveNaming from './index.js' -import { compareMessages, parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { compareMessages, joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' it('reports no errors on a project with no repetitive words in slices', () => { const root = parseIntoFsdRoot(` @@ -35,7 +35,9 @@ it('reports errors on a project with repetition of "page"', () => { `) const diagnostics = repetitiveNaming.check(root).diagnostics.sort(compareMessages) - expect(diagnostics).toEqual([{ message: 'Repetitive word "page" in slice names on layer "pages"' }]) + expect(diagnostics).toEqual([ + { message: 'Repetitive word "page" in slice names.', location: { path: joinFromRoot('pages') } }, + ]) }) it('recognizes words in different naming conventions', () => { @@ -53,7 +55,9 @@ it('recognizes words in different naming conventions', () => { `) const diagnostics = repetitiveNaming.check(root).diagnostics.sort(compareMessages) - expect(diagnostics).toEqual([{ message: 'Repetitive word "folder" in slice names on layer "entities"' }]) + expect(diagnostics).toEqual([ + { message: 'Repetitive word "folder" in slice names.', location: { path: joinFromRoot('entities') } }, + ]) }) it('does not complain about layers with just one slice', () => { diff --git a/packages/steiger-plugin-fsd/src/repetitive-naming/index.ts b/packages/steiger-plugin-fsd/src/repetitive-naming/index.ts index 8bc2eb7..85ac142 100644 --- a/packages/steiger-plugin-fsd/src/repetitive-naming/index.ts +++ b/packages/steiger-plugin-fsd/src/repetitive-naming/index.ts @@ -19,7 +19,7 @@ const repetitiveNaming = { check(root) { const diagnostics: Array = [] - for (const [layerName, layer] of Object.entries(getLayers(root))) { + for (const layer of Object.values(getLayers(root))) { if (!isSliced(layer)) { continue } @@ -39,7 +39,7 @@ const repetitiveNaming = { count >= sliceNames.length && wordsInSliceNames.every((words) => words.includes(word)) ) { - diagnostics.push({ message: `Repetitive word "${word}" in slice names on layer "${layerName}"` }) + diagnostics.push({ message: `Repetitive word "${word}" in slice names.`, location: { path: layer.path } }) } } } diff --git a/packages/steiger-plugin-fsd/src/segments-by-purpose/index.spec.ts b/packages/steiger-plugin-fsd/src/segments-by-purpose/index.spec.ts index b67400b..c29a6cb 100644 --- a/packages/steiger-plugin-fsd/src/segments-by-purpose/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/segments-by-purpose/index.spec.ts @@ -1,7 +1,7 @@ import { expect, it } from 'vitest' import segmentsByPurpose from './index.js' -import { compareMessages, parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { compareMessages, joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' it('reports no errors on a project with good segments', () => { const root = parseIntoFsdRoot(` @@ -50,10 +50,25 @@ it('reports errors on a project with bad segments', () => { const diagnostics = segmentsByPurpose.check(root).diagnostics.sort(compareMessages) expect(diagnostics).toEqual([ - { message: 'Non-descriptive segment name "components" on slice "user" on layer "entities"' }, - { message: 'Non-descriptive segment name "helpers" on layer "shared"' }, - { message: 'Non-descriptive segment name "hooks" on layer "shared"' }, - { message: 'Non-descriptive segment name "modals" on layer "shared"' }, - { message: 'Non-descriptive segment name "utils" on layer "shared"' }, + { + message: "This segment's name should describe the purpose of its contents, not what the contents are.", + location: { path: joinFromRoot('entities', 'user', 'components') }, + }, + { + message: "This segment's name should describe the purpose of its contents, not what the contents are.", + location: { path: joinFromRoot('shared', 'helpers') }, + }, + { + message: "This segment's name should describe the purpose of its contents, not what the contents are.", + location: { path: joinFromRoot('shared', 'hooks') }, + }, + { + message: "This segment's name should describe the purpose of its contents, not what the contents are.", + location: { path: joinFromRoot('shared', 'modals') }, + }, + { + message: "This segment's name should describe the purpose of its contents, not what the contents are.", + location: { path: joinFromRoot('shared', 'utils') }, + }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/segments-by-purpose/index.ts b/packages/steiger-plugin-fsd/src/segments-by-purpose/index.ts index 94df0fd..4771e3c 100644 --- a/packages/steiger-plugin-fsd/src/segments-by-purpose/index.ts +++ b/packages/steiger-plugin-fsd/src/segments-by-purpose/index.ts @@ -9,23 +9,27 @@ const segmentsByPurpose = { check(root) { const diagnostics: Array = [] - for (const [layerName, layer] of Object.entries(getLayers(root))) { + for (const layer of Object.values(getLayers(root))) { if (layer === null) { continue } if (!isSliced(layer)) { - for (const segmentName of Object.keys(getSegments(layer))) { + for (const [segmentName, segment] of Object.entries(getSegments(layer))) { if (BAD_NAMES.includes(segmentName)) { - diagnostics.push({ message: `Non-descriptive segment name "${segmentName}" on layer "${layerName}"` }) + diagnostics.push({ + message: "This segment's name should describe the purpose of its contents, not what the contents are.", + location: { path: segment.path }, + }) } } } else { - for (const [sliceName, slice] of Object.entries(getSlices(layer))) { - for (const segmentName of Object.keys(getSegments(slice))) { + for (const slice of Object.values(getSlices(layer))) { + for (const [segmentName, segment] of Object.entries(getSegments(slice))) { if (BAD_NAMES.includes(segmentName)) { diagnostics.push({ - message: `Non-descriptive segment name "${segmentName}" on slice "${sliceName}" on layer "${layerName}"`, + message: "This segment's name should describe the purpose of its contents, not what the contents are.", + location: { path: segment.path }, }) } } diff --git a/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.spec.ts b/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.spec.ts index 4744b47..0c1d00e 100644 --- a/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.spec.ts @@ -1,6 +1,6 @@ import { expect, it } from 'vitest' -import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' import excessiveSlicing from './index.js' it('reports no errors on projects with no shared/lib', () => { @@ -87,6 +87,7 @@ it('reports errors on a project with shared/lib above threshold', () => { expect(diagnostics).toEqual([ { message: 'Shared/lib has 19 modules, which is above the recommended threshold of 15. Consider grouping them.', + location: { path: joinFromRoot('shared', 'lib') }, }, ]) }) diff --git a/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.ts b/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.ts index 4a86ca9..a812cff 100644 --- a/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.ts +++ b/packages/steiger-plugin-fsd/src/shared-lib-grouping/index.ts @@ -22,6 +22,7 @@ const sharedLibGrouping = { if (lib.type === 'folder' && lib.children.length > THRESHOLD) { diagnostics.push({ message: `Shared/lib has ${lib.children.length} modules, which is above the recommended threshold of ${THRESHOLD}. Consider grouping them.`, + location: { path: lib.path }, }) } diff --git a/packages/types/index.ts b/packages/types/index.ts index cf15c47..ffb3987 100644 --- a/packages/types/index.ts +++ b/packages/types/index.ts @@ -26,6 +26,12 @@ export interface RuleResult { export interface Diagnostic { message: string fixes?: Array + location: { + /** Absolute path to a folder or a file that contains the issue. */ + path: string + line?: number + column?: number + } } export type Fix =