Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure forbidden-imports rule checks files directly inside layers #64

Merged
merged 10 commits into from
Jul 25, 2024
Merged

Make sure forbidden-imports rule checks files directly inside layers #64

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b4dde41
Make sure forbidden-imports rule checks files directly inside layers
daniilsapa Jul 21, 2024
935407c
Add a change log
daniilsapa Jul 21, 2024
d8c063d
Fix the problem of traversing the layer filesystem tree 2 times
daniilsapa Jul 22, 2024
92ed506
Fix constant declaration placement
daniilsapa Jul 23, 2024
737ed42
Fix the positive test case for forbidden-imports
daniilsapa Jul 23, 2024
892f5de
Cover changes in indexSourceFiles with tests
daniilsapa Jul 23, 2024
249e5b2
Improve parseIntoFsdRoot function by introducing the capability to sp…
daniilsapa Jul 24, 2024
6d512bc
Use the optional rootPath argument instead of directly accessing chil…
daniilsapa Jul 24, 2024
5731cc9
Fix the problem with failing test on Windows
daniilsapa Jul 24, 2024
74c02d1
Simplify parseIntoFsdRoot test utility
daniilsapa Jul 25, 2024
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
32 changes: 32 additions & 0 deletions packages/steiger-plugin-fsd/src/_lib/index-source-files.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ if (import.meta.vitest) {
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
📂 app
📂 ui
📄 index.ts
📄 root.ts
📄 index.ts
`)

expect(indexSourceFiles(root)).toEqual({
Expand Down Expand Up @@ -170,6 +175,33 @@ if (import.meta.vitest) {
segmentName: 'ui',
sliceName: null,
},
[joinFromRoot('app', 'ui', 'index.ts')]: {
file: {
path: joinFromRoot('app', 'ui', 'index.ts'),
type: 'file',
},
layerName: 'app',
segmentName: 'ui',
sliceName: null,
},
[joinFromRoot('app', 'root.ts')]: {
file: {
path: joinFromRoot('app', 'root.ts'),
type: 'file',
},
layerName: 'app',
segmentName: 'root',
sliceName: null,
},
[joinFromRoot('app', 'index.ts')]: {
file: {
path: joinFromRoot('app', 'index.ts'),
type: 'file',
},
layerName: 'app',
segmentName: null,
sliceName: null,
},
})
})
}
105 changes: 103 additions & 2 deletions packages/steiger-plugin-fsd/src/_lib/prepare-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,31 @@ import type { FsdRoot } from '@feature-sliced/filesystem'
import type { Folder, File, Diagnostic } from '@steiger/types'
import { vi } from 'vitest'

function findSubfolder(folder: Folder, path: string): Folder {
function checkFolder(folder: Folder): Folder {
if (folder.path === path) {
return folder
}

if (path.startsWith(folder.path)) {
for (const child of folder.children) {
if (child.type === 'folder') {
const result = checkFolder(child)
if (result) {
return result
}
}
}
}

throw new Error(`Path "${path}" not found in the provided file system mock!`)
}

return checkFolder(folder)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: I meant a more simple solution — you pass the root path as the second argument, and whatever file tree you write with emojis is "mounted" to that root. For example:

parseIntoFsdRoot(`
  📂 test
    📄 file
`, "/home/illright");  // the "file" becomes `/home/illright/test/file`

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree, the simpler the better, the KISS principle should not be forgotten 😂


/** Parse a multi-line indented string with emojis for files and folders into an FSD root. */
export function parseIntoFsdRoot(fsMarkup: string): FsdRoot {
export function parseIntoFsdRoot(fsMarkup: string, rootPath?: string): FsdRoot {
function parseFolder(lines: Array<string>, path: string): Folder {
const children: Array<Folder | File> = []

Expand All @@ -32,8 +55,9 @@ export function parseIntoFsdRoot(fsMarkup: string): FsdRoot {
.filter(Boolean)
.map((line, _i, lines) => line.slice(lines[0].search(/\S/)))
.filter(Boolean)
const parsedFolder = parseFolder(lines, joinFromRoot())

return parseFolder(lines, joinFromRoot())
return rootPath ? findSubfolder(parsedFolder, rootPath) : parsedFolder
}

export function compareMessages(a: Diagnostic, b: Diagnostic): number {
Expand Down Expand Up @@ -149,4 +173,81 @@ if (import.meta.vitest) {
],
})
})

test('it should return a nested root folder when the optional rootPath argument is passed', () => {
const markup = `
📂 src
📂 entities
📂 users
📂 ui
📄 index.ts
📂 posts
📂 ui
📄 index.ts
📂 shared
📂 ui
📄 index.ts
📄 Button.tsx
`
const root = parseIntoFsdRoot(markup, joinFromRoot('src', 'entities'))

expect(root).toEqual({
type: 'folder',
path: joinFromRoot('src', 'entities'),
children: [
{
type: 'folder',
path: joinFromRoot('src', 'entities', 'users'),
children: [
{
type: 'folder',
path: joinFromRoot('src', 'entities', 'users', 'ui'),
children: [],
},
{
type: 'file',
path: joinFromRoot('src', 'entities', 'users', 'index.ts'),
},
],
},
{
type: 'folder',
path: joinFromRoot('src', 'entities', 'posts'),
children: [
{
type: 'folder',
path: joinFromRoot('src', 'entities', 'posts', 'ui'),
children: [],
},
{
type: 'file',
path: joinFromRoot('src', 'entities', 'posts', 'index.ts'),
},
],
},
],
})
})

test('it should throw an error when the path (from rootPath argument) is not found in the provided file system mock', () => {
const markup = `
📂 src
📂 entities
📂 users
📂 ui
📄 index.ts
📂 posts
📂 ui
📄 index.ts
📂 shared
📂 ui
📄 index.ts
📄 Button.tsx
`
const nonExistentPath = joinFromRoot('src', 'non-existent-folder')

expect(() => parseIntoFsdRoot(markup, nonExistentPath)).toThrowError(
`Path "${nonExistentPath}" not found in the provided file system mock!`,
)
})
}
183 changes: 97 additions & 86 deletions packages/steiger-plugin-fsd/src/forbidden-imports/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { expect, it, vi } from 'vitest'
import { Folder } from '@steiger/types'

import { joinFromRoot, parseIntoFsdRoot } from '../_lib/prepare-test.js'
import forbiddenImports from './index.js'
Expand Down Expand Up @@ -56,52 +55,58 @@ vi.mock('node:fs', async (importOriginal) => {
})

it('reports no errors on a project with only correct imports', async () => {
const root = parseIntoFsdRoot(`
📂 src
📂 shared
📂 ui
📄 styles.ts
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 pages
📂 editor
const root = parseIntoFsdRoot(
`
📂 src
📂 shared
📂 ui
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
`)
📄 styles.ts
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 pages
📂 editor
📂 ui
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
`,
joinFromRoot('src'),
)

expect((await forbiddenImports.check(root)).diagnostics).toEqual([])
})

it('reports errors on a project with cross-imports in entities', async () => {
const root = parseIntoFsdRoot(`
📂 src
📂 shared
📂 ui
📄 styles.ts
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 entities
📂 user
const root = parseIntoFsdRoot(
`
📂 src
📂 shared
📂 ui
📄 UserAvatar.tsx
📄 index.ts
📂 product
📂 ui
📄 ProductCard.tsx
📄 index.ts
📂 pages
📂 editor
📂 ui
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
`)
📄 styles.ts
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 entities
📂 user
📂 ui
📄 UserAvatar.tsx
📄 index.ts
📂 product
📂 ui
📄 ProductCard.tsx
📄 index.ts
📂 pages
📂 editor
📂 ui
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
`,
joinFromRoot('src'),
)

expect((await forbiddenImports.check(root.children[0] as Folder)).diagnostics).toEqual([
expect((await forbiddenImports.check(root)).diagnostics).toEqual([
{
message: `Forbidden cross-import from slice "user".`,
location: { path: joinFromRoot('src', 'entities', 'product', 'ui', 'ProductCard.tsx') },
Expand All @@ -110,29 +115,32 @@ it('reports errors on a project with cross-imports in entities', async () => {
})

it('reports errors on a project where a feature imports from a page', async () => {
const root = parseIntoFsdRoot(`
📂 src
📂 shared
📂 ui
📄 styles.ts
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 features
📂 comments
📂 ui
📄 CommentCard.tsx
📄 index.ts
📂 pages
📂 editor
const root = parseIntoFsdRoot(
`
📂 src
📂 shared
📂 ui
📄 styles.ts
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
`)
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 features
📂 comments
📂 ui
📄 CommentCard.tsx
📄 index.ts
📂 pages
📂 editor
📂 ui
📄 styles.ts
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
`,
joinFromRoot('src'),
)

expect((await forbiddenImports.check(root.children[0] as Folder)).diagnostics.sort()).toEqual([
expect((await forbiddenImports.check(root)).diagnostics.sort()).toEqual([
{
message: `Forbidden import from higher layer "pages".`,
location: { path: joinFromRoot('src', 'features', 'comments', 'ui', 'CommentCard.tsx') },
Expand All @@ -141,37 +149,40 @@ it('reports errors on a project where a feature imports from a page', async () =
})

it('reports errors in a project where a lower level imports from files that are direct children of a higher level', async () => {
const root = parseIntoFsdRoot(`
📂 src
📂 shared
📂 ui
📄 styles.ts
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 entities
📂 cart
📄 index.ts
📂 lib
📄 count-cart-items.ts
📄 index.ts
📂 ui
📄 SmallCart.tsx
📂 pages
📂 editor
const root = parseIntoFsdRoot(
`
📂 src
📂 shared
📂 ui
📄 styles.ts
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
📂 app
📂 ui
📄 Button.tsx
📄 TextField.tsx
📄 index.ts
📂 entities
📂 cart
📄 index.ts
📂 lib
📄 count-cart-items.ts
📄 index.ts
📂 ui
📄 SmallCart.tsx
📂 pages
📂 editor
📂 ui
📄 styles.ts
📄 EditorPage.tsx
📄 Editor.tsx
📄 index.ts
📂 app
📂 ui
📄 index.ts
📄 index.ts
📄 index.ts
📄 root.ts
`)
📄 root.ts
`,
joinFromRoot('src'),
)

const diagnostics = (await forbiddenImports.check(root.children[0] as Folder)).diagnostics
const diagnostics = (await forbiddenImports.check(root)).diagnostics
expect(diagnostics).toEqual([
{
message: `Forbidden import from higher layer "app".`,
Expand Down
Loading