From 774398b34cdc2a9c1e47255d78a29dbe99db10ac Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Sat, 13 Jul 2024 18:38:11 +0300 Subject: [PATCH 01/11] Implement no-segments-on-sliced-layers rule and cover it with unit tests --- .../index.spec.ts | 101 ++++++++++++++++++ .../src/no-segments-on-sliced-layers/index.ts | 32 ++++++ 2 files changed, 133 insertions(+) create mode 100644 packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts create mode 100644 packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts new file mode 100644 index 0000000..4f73b24 --- /dev/null +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from 'vitest' + +import noSegmentsOnSlicedLayers from './index.js' +import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' + +describe('no-segments-on-sliced-layers rule', () => { + it('reports no errors on a project where the sliced layers has no segments in direct children', () => { + const root = parseIntoFsdRoot(` + 📂 shared + 📂 ui + 📄 index.ts + 📂 i18n + 📄 index.ts + 📂 entities + 📂 user + 📂 ui + 📄 Name.tsx + 📂 api + 📄 useCurrentUser.ts + 📄 index.ts + 📂 document + 📂 api + 📄 useDocument.ts + 📂 pages + 📂 home + 📂 ui + 📄 index.ts + `) + + console.log(noSegmentsOnSlicedLayers.check(root)) + + expect(noSegmentsOnSlicedLayers.check(root)).toEqual({ diagnostics: [] }) + }) + + it('reports errors on a project where the "Entities" layer has segments in direct children', () => { + const root = parseIntoFsdRoot(` + 📂 shared + 📂 ui + 📄 index.ts + 📂 i18n + 📄 index.ts + 📂 entities + 📂 user + 📄 index.ts + 📄 Name.tsx + 📂 ui + 📄 index.ts + 📂 features + 📂 user + 📂 ui + 📄 LogIn.tsx + 📄 index.ts + 📄 index.ts + 📂 api + 📄 index.ts + 📂 widgets + 📂 footer + 📂 ui + 📄 Footer.tsx + 📄 index.ts + 📄 index.ts + 📂 config + 📄 index.ts + 📂 pages + 📂 home + 📂 ui + 📄 index.ts + 📂 settings + 📂 profile + 📄 ProfilePage.tsx + 📄 index.ts + 📂 lib + 📄 index.ts + `) + + const diagnostics = noSegmentsOnSlicedLayers.check(root).diagnostics + console.log(diagnostics) + expect(diagnostics).toEqual([ + { + message: + 'Conventional segment "ui" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', + location: { path: joinFromRoot('entities', 'ui') }, + }, + { + message: + 'Conventional segment "api" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', + location: { path: joinFromRoot('features', 'api') }, + }, + { + message: + 'Conventional segment "config" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', + location: { path: joinFromRoot('widgets', 'config') }, + }, + { + message: + 'Conventional segment "lib" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', + location: { path: joinFromRoot('pages', 'lib') }, + }, + ]) + }) +}) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts new file mode 100644 index 0000000..d58fe0b --- /dev/null +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts @@ -0,0 +1,32 @@ +import { getLayers, isSliced, conventionalSegmentNames } from '@feature-sliced/filesystem' +import { type Diagnostic, Rule } from '@steiger/types' + +const noSegmentsOnSlicedLayers = { + name: 'no-segments-on-sliced-layers', + check(root) { + const diagnostics: Array = [] + const layers = Object.values(getLayers(root)) + + for (const layer of layers) { + if (isSliced(layer)) { + for (const directChild of layer.children) { + if (directChild.type === 'folder') { + const folderName = directChild.path.split('/').pop() + const isConventionalSegment = folderName && conventionalSegmentNames.includes(folderName) + + if (isConventionalSegment) { + diagnostics.push({ + message: `Conventional segment "${folderName}" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.`, + location: { path: directChild.path }, + }) + } + } + } + } + } + + return { diagnostics } + }, +} satisfies Rule + +export default noSegmentsOnSlicedLayers From 067634e0c83ae3f12caa3ec99d85741cf5fd2ab3 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Sat, 13 Jul 2024 18:44:48 +0300 Subject: [PATCH 02/11] Connect no-segments-on-sliced-layers rule to the app, add documents for the new rule --- README.md | 1 + packages/steiger-plugin-fsd/src/index.ts | 2 + .../no-segments-on-sliced-layers/README.md | 47 +++++++++++++++++++ packages/steiger/README.md | 1 + packages/steiger/src/models/config.ts | 1 + 5 files changed, 52 insertions(+) create mode 100644 packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md diff --git a/README.md b/README.md index aa2be9a..28e71af 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,7 @@ Currently, Steiger is not extendable with more rules, though that will change in no-public-api-sidestep Forbid going around the public API of a slice to import directly from an internal module in a slice. no-reserved-folder-names Forbid subfolders in segments that have the same name as other conventional segments. no-segmentless-slices Forbid slices that don't have any segments. + no-segments-on-sliced-layers Forbid segments (like ui, lib, api ...) that appear directly in sliced layer folders (entities, features, ...) public-api Require slices (and segments on sliceless layers like Shared) to have a public API definition. repetitive-naming Ensure that all entities are named consistently in terms of pluralization. segments-by-purpose Discourage the use of segment names that group code by its essence, and instead encourage grouping by purpose diff --git a/packages/steiger-plugin-fsd/src/index.ts b/packages/steiger-plugin-fsd/src/index.ts index 6e63f8b..8382722 100644 --- a/packages/steiger-plugin-fsd/src/index.ts +++ b/packages/steiger-plugin-fsd/src/index.ts @@ -7,6 +7,7 @@ import noLayerPublicApi from './no-layer-public-api/index.js' import noPublicApiSidestep from './no-public-api-sidestep/index.js' import noReservedFolderNames from './no-reserved-folder-names/index.js' import noSegmentlessSlices from './no-segmentless-slices/index.js' +import noSegmentsOnSlicedLayers from './no-segments-on-sliced-layers/index.js' import publicApi from './public-api/index.js' import repetitiveNaming from './repetitive-naming/index.js' import segmentsByPurpose from './segments-by-purpose/index.js' @@ -23,6 +24,7 @@ export default [ noPublicApiSidestep, noReservedFolderNames, noSegmentlessSlices, + noSegmentsOnSlicedLayers, publicApi, repetitiveNaming, segmentsByPurpose, diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md new file mode 100644 index 0000000..df6463c --- /dev/null +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md @@ -0,0 +1,47 @@ +# `no-segmentless-slices` + +Forbid segments that appear in direct children of sliced layers. + +Examples of project structures that pass this rule: + +``` +📂 shared + 📂 ui + 📄 index.ts + 📂 lib + 📄 index.ts +📂 entities + 📂 user + 📂 ui + 📂 model + 📄 index.ts +📂 pages + 📂 home + 📂 ui + 📄 index.ts +``` + +Examples of project structures that fail this rule: + +``` +📂 shared + 📂 ui + 📄 index.ts + 📂 lib + 📄 index.ts +📂 entities + 📂 user + 📂 ui + 📂 model + 📄 index.ts + 📂 api // ❌ + 📄 index.ts +📂 pages + 📂 home + 📂 ui + 📄 index.ts +``` + +## Rationale + +Slices exist to partition code by business domain and entities. You can freely create and name them (e.g. `pages` home, profile and `entities` user, product, ...) based on your needs, application logic, company glossary, etc. Slices contain code of different type/purposes (segments) to implement their part of functionality. Segments (`ui`, `lib`, `api`) are simply a division of code by purpose, thus "ownerless" segments in sliced layers are not allowed since they need to be attached to some part of the business domain inside these layers. diff --git a/packages/steiger/README.md b/packages/steiger/README.md index aa2be9a..28e71af 100644 --- a/packages/steiger/README.md +++ b/packages/steiger/README.md @@ -66,6 +66,7 @@ Currently, Steiger is not extendable with more rules, though that will change in no-public-api-sidestep Forbid going around the public API of a slice to import directly from an internal module in a slice. no-reserved-folder-names Forbid subfolders in segments that have the same name as other conventional segments. no-segmentless-slices Forbid slices that don't have any segments. + no-segments-on-sliced-layers Forbid segments (like ui, lib, api ...) that appear directly in sliced layer folders (entities, features, ...) public-api Require slices (and segments on sliceless layers like Shared) to have a public API definition. repetitive-naming Ensure that all entities are named consistently in terms of pluralization. segments-by-purpose Discourage the use of segment names that group code by its essence, and instead encourage grouping by purpose diff --git a/packages/steiger/src/models/config.ts b/packages/steiger/src/models/config.ts index c9aee7b..8241e07 100644 --- a/packages/steiger/src/models/config.ts +++ b/packages/steiger/src/models/config.ts @@ -14,6 +14,7 @@ export const schema = z.object({ 'no-public-api-sidestep', 'no-reserved-folder-names', 'no-segmentless-slices', + 'no-segments-on-sliced-layers', 'public-api', 'repetitive-naming', 'segments-by-purpose', From b359c14117e08a75437352683cc9cc6eed661cbe Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Sat, 13 Jul 2024 18:54:22 +0300 Subject: [PATCH 03/11] Clean up --- .../src/no-segments-on-sliced-layers/index.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts index 4f73b24..d3f9ba5 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts @@ -27,8 +27,6 @@ describe('no-segments-on-sliced-layers rule', () => { 📄 index.ts `) - console.log(noSegmentsOnSlicedLayers.check(root)) - expect(noSegmentsOnSlicedLayers.check(root)).toEqual({ diagnostics: [] }) }) @@ -74,7 +72,7 @@ describe('no-segments-on-sliced-layers rule', () => { `) const diagnostics = noSegmentsOnSlicedLayers.check(root).diagnostics - console.log(diagnostics) + expect(diagnostics).toEqual([ { message: From 36765d49813c84a94928fa4476b9fab16d2fc363 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Sat, 13 Jul 2024 18:55:30 +0300 Subject: [PATCH 04/11] Add a changelog --- .changeset/mean-pianos-cover.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/mean-pianos-cover.md diff --git a/.changeset/mean-pianos-cover.md b/.changeset/mean-pianos-cover.md new file mode 100644 index 0000000..82d4a5a --- /dev/null +++ b/.changeset/mean-pianos-cover.md @@ -0,0 +1,6 @@ +--- +'@feature-sliced/steiger-plugin': minor +'steiger': minor +--- + +Add no-segments-on-sliced-layers rule From 5509bc221bda513654b5c9df45896ff338ab5824 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Sat, 13 Jul 2024 22:44:31 +0300 Subject: [PATCH 05/11] Fix the issue with an incorrect path separator on Windows --- .../src/no-segments-on-sliced-layers/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts index d58fe0b..f2da424 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts @@ -1,5 +1,7 @@ +import { sep } from 'node:path' + import { getLayers, isSliced, conventionalSegmentNames } from '@feature-sliced/filesystem' -import { type Diagnostic, Rule } from '@steiger/types' +import type { Diagnostic, Rule } from '@steiger/types' const noSegmentsOnSlicedLayers = { name: 'no-segments-on-sliced-layers', @@ -11,7 +13,7 @@ const noSegmentsOnSlicedLayers = { if (isSliced(layer)) { for (const directChild of layer.children) { if (directChild.type === 'folder') { - const folderName = directChild.path.split('/').pop() + const folderName = directChild.path.split(sep).pop() const isConventionalSegment = folderName && conventionalSegmentNames.includes(folderName) if (isConventionalSegment) { From d5a54cbb7045b34d9b92cd356fa2e686b8c022c3 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Sat, 13 Jul 2024 23:09:42 +0300 Subject: [PATCH 06/11] Fix the title of the negative test case --- .../src/no-segments-on-sliced-layers/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts index d3f9ba5..cfc6efc 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts @@ -30,7 +30,7 @@ describe('no-segments-on-sliced-layers rule', () => { expect(noSegmentsOnSlicedLayers.check(root)).toEqual({ diagnostics: [] }) }) - it('reports errors on a project where the "Entities" layer has segments in direct children', () => { + it('reports errors on a project where a sliced layer has segments among its direct children', () => { const root = parseIntoFsdRoot(` 📂 shared 📂 ui From a10c893a093a58756be73bd2a94c569ee9444f85 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Mon, 15 Jul 2024 11:52:40 +0300 Subject: [PATCH 07/11] Change rationale for no-segments-on-sliced-layers rule Co-authored-by: Lev Chelyadinov --- .../src/no-segments-on-sliced-layers/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md index df6463c..b1f3042 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md @@ -44,4 +44,4 @@ Examples of project structures that fail this rule: ## Rationale -Slices exist to partition code by business domain and entities. You can freely create and name them (e.g. `pages` home, profile and `entities` user, product, ...) based on your needs, application logic, company glossary, etc. Slices contain code of different type/purposes (segments) to implement their part of functionality. Segments (`ui`, `lib`, `api`) are simply a division of code by purpose, thus "ownerless" segments in sliced layers are not allowed since they need to be attached to some part of the business domain inside these layers. +Several folder names like `ui` or `api` are conventionally understood in FSD as segments. When you have segments as direct children, it's either because that's the name you chose for your slice, or because some code ended up unsliced. In the first case, where that is the name of your slice, such a name is likely to cause confusion among developers who are familiar with FSD. In the second case, where a segment on a layer is because there was no slice to put it in, it's a violation of FSD structure, which decreases the benefits of FSD and also causes confusion. From e5acd3d328bf12c74197b106a9ac6bbaf4bf9f3f Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Mon, 15 Jul 2024 12:13:59 +0300 Subject: [PATCH 08/11] Rephrase the error message for clarity Co-authored-by: Lev Chelyadinov --- .../src/no-segments-on-sliced-layers/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts index f2da424..38d8050 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts @@ -18,7 +18,7 @@ const noSegmentsOnSlicedLayers = { if (isConventionalSegment) { diagnostics.push({ - message: `Conventional segment "${folderName}" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.`, + message: `Conventional segment "${folderName}" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.`, location: { path: directChild.path }, }) } From 4f66a1e95e14c17b7f545d820d40973242f4c847 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Mon, 15 Jul 2024 12:15:27 +0300 Subject: [PATCH 09/11] Delegate getting the folder name to path module --- .../src/no-segments-on-sliced-layers/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts index 38d8050..b04a46f 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.ts @@ -1,4 +1,4 @@ -import { sep } from 'node:path' +import { basename } from 'node:path' import { getLayers, isSliced, conventionalSegmentNames } from '@feature-sliced/filesystem' import type { Diagnostic, Rule } from '@steiger/types' @@ -13,7 +13,7 @@ const noSegmentsOnSlicedLayers = { if (isSliced(layer)) { for (const directChild of layer.children) { if (directChild.type === 'folder') { - const folderName = directChild.path.split(sep).pop() + const folderName = basename(directChild.path) const isConventionalSegment = folderName && conventionalSegmentNames.includes(folderName) if (isConventionalSegment) { From 2540ff6f12ff0380196eb7a0d73ad9873ae41739 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Mon, 15 Jul 2024 12:27:08 +0300 Subject: [PATCH 10/11] Sort diagnostics in the test case to avoid false negatives in the future --- .../index.spec.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts index cfc6efc..098ce1e 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import noSegmentsOnSlicedLayers from './index.js' -import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js' +import { joinFromRoot, parseIntoFsdRoot, compareMessages } from '../_lib/prepare-test.js' describe('no-segments-on-sliced-layers rule', () => { it('reports no errors on a project where the sliced layers has no segments in direct children', () => { @@ -71,29 +71,29 @@ describe('no-segments-on-sliced-layers rule', () => { 📄 index.ts `) - const diagnostics = noSegmentsOnSlicedLayers.check(root).diagnostics + const diagnostics = noSegmentsOnSlicedLayers.check(root).diagnostics.toSorted(compareMessages) expect(diagnostics).toEqual([ { message: - 'Conventional segment "ui" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', - location: { path: joinFromRoot('entities', 'ui') }, - }, - { - message: - 'Conventional segment "api" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', + 'Conventional segment "api" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.', location: { path: joinFromRoot('features', 'api') }, }, { message: - 'Conventional segment "config" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', + 'Conventional segment "config" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.', location: { path: joinFromRoot('widgets', 'config') }, }, { message: - 'Conventional segment "lib" found as a direct child of a sliced layer. Consider moving it inside a slice or to another layer.', + 'Conventional segment "lib" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.', location: { path: joinFromRoot('pages', 'lib') }, }, + { + message: + 'Conventional segment "ui" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.', + location: { path: joinFromRoot('entities', 'ui') }, + }, ]) }) }) From 81d68e9a91f64c1d85cc9ea9d06be8095229fb79 Mon Sep 17 00:00:00 2001 From: Daniil Sapa Date: Tue, 16 Jul 2024 16:41:09 +0300 Subject: [PATCH 11/11] Replace toSorted with sort for to support Node v18 --- .../src/no-segments-on-sliced-layers/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts index 098ce1e..113110a 100644 --- a/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts +++ b/packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/index.spec.ts @@ -71,7 +71,7 @@ describe('no-segments-on-sliced-layers rule', () => { 📄 index.ts `) - const diagnostics = noSegmentsOnSlicedLayers.check(root).diagnostics.toSorted(compareMessages) + const diagnostics = noSegmentsOnSlicedLayers.check(root).diagnostics.sort(compareMessages) expect(diagnostics).toEqual([ {