Add location information to the diagnostic and refactor the rules to …

…supply it

.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

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<string>): Record<string, Array<string>> {
   const groups: Record<string, Array<string>> = {}
 

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<string>) {

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.

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') },
+    },
   ])
 })

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 | undefined>(
+          (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 },
           })
         }
       }

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.',
+    },
+  ])
+})

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,17 +18,27 @@ const excessiveSlicing = {
     const diagnostics: Array<Diagnostic> = []
 
     for (const [layerName, layer] of Object.entries(getLayers(root))) {
-      if (!isSliced(layer)) {
+      if (!isSliced(layer) || !(layerName in THRESHOLDS)) {
         continue
       }
 
       const sliceGroups = groupSlices(Object.keys(getSlices(layer)))
       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) },
           })
         }
       }

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') },
     },
   ])
 })