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.

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

feat: extend the functionality of inconsistent-naming #166

Open
wants to merge 2 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rude-items-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@feature-sliced/steiger-plugin': minor
---

Extend the functionality of `inconsistent-naming` to support multi-word names and handle rename collisions
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/** Extracts individual words in every naming scheme. */
export const wordPattern = /([A-Z0-9]{2,}(?![A-Z][a-z])|[A-Z]?[a-z0-9]+)/g

/**
* Extract the main subject in a multi-word subject name.
*
* @example
* getMainSubject("a book with pages") // "book"
* getMainSubject("admin-users") // "users"
* getMainSubject("receiptsByOrder") // "receipts"
*/
export function getMainSubject(name: string) {
const words = [...name.matchAll(wordPattern)]
.map((match) => match[0])
.filter((word) => !articles.includes(word.toLocaleLowerCase()))

const prepositionIndex = words.findIndex((word) => prepositions.includes(word.toLocaleLowerCase()))
if (prepositionIndex === -1) {
return words[words.length - 1]
}
const mainPart = words.slice(0, prepositionIndex)
return mainPart[mainPart.length - 1]
}

if (import.meta.vitest) {
const { test, expect } = import.meta.vitest

test('getMainSubject', () => {
expect(getMainSubject('a book with pages')).toEqual('book')
expect(getMainSubject('admin-users')).toEqual('users')
expect(getMainSubject('receiptsByOrder')).toEqual('receipts')
})
}

const articles = ['a', 'an', 'the']
const prepositions = [
'about',
'above',
'across',
'after',
'against',
'along',
'among',
'around',
'at',
'before',
'behind',
'below',
'beneath',
'beside',
'besides',
'between',
'beyond',
'but',
'by',
'concerning',
'despite',
'down',
'during',
'except',
'excepting',
'for',
'from',
'in',
'inside',
'into',
'like',
'near',
'of',
'off',
'on',
'onto',
'out',
'outside',
'over',
'past',
'regarding',
'since',
'through',
'throughout',
'to',
'toward',
'under',
'underneath',
'until',
'up',
'upon',
'with',
'within',
'without',
]
95 changes: 85 additions & 10 deletions packages/steiger-plugin-fsd/src/inconsistent-naming/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { expect, it } from 'vitest'

import { compareMessages, joinFromRoot, parseIntoFolder as parseIntoFsdRoot } from '@steiger/toolkit'
import { joinFromRoot, parseIntoFolder as parseIntoFsdRoot } from '@steiger/toolkit'
import inconsistentNaming from './index.js'

it('reports no errors on slice names that are pluralized consistently', () => {
const root = parseIntoFsdRoot(
it('reports no errors on entity names that are pluralized consistently', () => {
const root1 = parseIntoFsdRoot(
`
📂 entities
📂 users
Expand All @@ -16,11 +16,41 @@ it('reports no errors on slice names that are pluralized consistently', () => {
`,
joinFromRoot('users', 'user', 'project', 'src'),
)
const root2 = parseIntoFsdRoot(
`
📂 entities
📂 user
📂 ui
📄 index.ts
📂 post
📂 ui
📄 index.ts
`,
joinFromRoot('users', 'user', 'project', 'src'),
)

expect(inconsistentNaming.check(root1)).toEqual({ diagnostics: [] })
expect(inconsistentNaming.check(root2)).toEqual({ diagnostics: [] })
})

it('reports no errors on multi-word entity names that are pluralized consistently', () => {
const root = parseIntoFsdRoot(
`
📂 entities
📂 admin-users
📂 ui
📄 index.ts
📂 employers-of-record
📂 ui
📄 index.ts
`,
joinFromRoot('users', 'user', 'project', 'src'),
)

expect(inconsistentNaming.check(root)).toEqual({ diagnostics: [] })
})

it('reports an error on slice names that are not pluralized consistently', () => {
it('reports an error on entity names that are not pluralized consistently', () => {
const root = parseIntoFsdRoot(
`
📂 entities
Expand All @@ -34,10 +64,10 @@ it('reports an error on slice names that are not pluralized consistently', () =>
joinFromRoot('users', 'user', 'project', 'src'),
)

const diagnostics = inconsistentNaming.check(root).diagnostics.sort(compareMessages)
const diagnostics = inconsistentNaming.check(root).diagnostics
expect(diagnostics).toEqual([
{
message: 'Inconsistent pluralization of slice names. Prefer all plural names',
message: 'Inconsistent pluralization of entity names. Prefer all plural names.',
fixes: [
{
type: 'rename',
Expand All @@ -54,10 +84,10 @@ it('prefers the singular form when there are more singular slices', () => {
const root = parseIntoFsdRoot(
`
📂 entities
📂 user
📂 admin-user
📂 ui
📄 index.ts
📂 post
📂 news-post
📂 ui
📄 index.ts
📂 comments
Expand All @@ -67,10 +97,10 @@ it('prefers the singular form when there are more singular slices', () => {
joinFromRoot('users', 'user', 'project', 'src'),
)

const diagnostics = inconsistentNaming.check(root).diagnostics.sort(compareMessages)
const diagnostics = inconsistentNaming.check(root).diagnostics
expect(diagnostics).toEqual([
{
message: 'Inconsistent pluralization of slice names. Prefer all singular names',
message: 'Inconsistent pluralization of entity names. Prefer all singular names.',
fixes: [
{
type: 'rename',
Expand All @@ -82,3 +112,48 @@ it('prefers the singular form when there are more singular slices', () => {
},
])
})

it('recognizes the special case when there are two pluralizations of the same name', () => {
const root = parseIntoFsdRoot(
`
📂 entities
📂 admin-user
📂 ui
📄 index.ts
📂 admin-users
📂 ui
📄 index.ts
`,
joinFromRoot('users', 'user', 'project', 'src'),
)

const diagnostics = inconsistentNaming.check(root).diagnostics
expect(diagnostics).toEqual([
{
message: 'Avoid having both "admin-user" and "admin-users" entities.',
location: { path: joinFromRoot('users', 'user', 'project', 'src', 'entities', 'admin-user') },
},
])
})

it('allows inconsistency between different slice groups', () => {
const root = parseIntoFsdRoot(
`
📂 entities
📂 admin-user
📂 ui
📄 index.ts
📂 group
📄 index.ts
📂 post-parts
📂 posts
📄 index.ts
📂 authors
📄 index.ts
`,
joinFromRoot('users', 'user', 'project', 'src'),
)

const diagnostics = inconsistentNaming.check(root).diagnostics
expect(diagnostics).toEqual([])
})
63 changes: 46 additions & 17 deletions packages/steiger-plugin-fsd/src/inconsistent-naming/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import type { PartialDiagnostic, Rule } from '@steiger/toolkit'

import { groupSlices } from '../_lib/group-slices.js'
import { NAMESPACE } from '../constants.js'
import { getMainSubject } from './get-main-subject.js'

/** Detect inconsistent naming of slices on layers (singular vs plural) */
/** Detect inconsistent pluralization of entities. */
const inconsistentNaming = {
name: `${NAMESPACE}/inconsistent-naming` as const,
check(root) {
Expand All @@ -22,29 +23,57 @@ const inconsistentNaming = {
const slices = getSlices(entities)
const sliceNames = groupSlices(Object.keys(slices))
for (const [groupPrefix, group] of Object.entries(sliceNames)) {
const [pluralNames, singularNames] = partition(group, isPlural)
const [pluralNames, singularNames] = partition(
group.map((name) => [name, getMainSubject(name)] as const),
([, mainSubject]) => isPlural(mainSubject),
)

/** Names that exist in both singular and plural forms for filtering later. */
const duplicates = {
singular: [] as Array<string>,
plural: [] as Array<string>,
}

if (pluralNames.length > 0 && singularNames.length > 0) {
const message = 'Inconsistent pluralization of slice names'
for (const [singularName, mainSubject] of singularNames) {
const pluralized = singularName.replace(mainSubject, plural(mainSubject))
if (group.includes(pluralized)) {
duplicates.singular.push(singularName)
duplicates.plural.push(pluralized)

diagnostics.push({
message: `Avoid having both "${singularName}" and "${plural(singularName)}" entities${groupPrefix === '' ? '' : ' in the same slice group'}.`,
location: { path: join(entities.path, groupPrefix, singularName) },
})
}
}

if (pluralNames.length >= singularNames.length) {
const message = 'Inconsistent pluralization of entity names'
if (
pluralNames.length >= singularNames.length &&
singularNames.some(([name]) => !duplicates.singular.includes(name))
) {
diagnostics.push({
message: `${message}. Prefer all plural names`,
fixes: singularNames.map((name) => ({
type: 'rename',
path: join(entities.path, groupPrefix, name),
newName: plural(name),
})),
message: `${message}. Prefer all plural names.`,
fixes: singularNames
.filter(([name]) => !duplicates.singular.includes(name))
.map(([name, mainWord]) => ({
type: 'rename',
path: join(entities.path, groupPrefix, name),
newName: name.replace(mainWord, plural(mainWord)),
})),
location: { path: join(entities.path, groupPrefix) },
})
} else {
} else if (pluralNames.some(([name]) => !duplicates.plural.includes(name))) {
diagnostics.push({
message: `${message}. Prefer all singular names`,
fixes: pluralNames.map((name) => ({
type: 'rename',
path: join(entities.path, groupPrefix, name),
newName: singular(name),
})),
message: `${message}. Prefer all singular names.`,
fixes: pluralNames
.filter(([name]) => !duplicates.plural.includes(name))
.map(([name, mainWord]) => ({
type: 'rename',
path: join(entities.path, groupPrefix, name),
newName: name.replace(mainWord, singular(mainWord)),
})),
location: { path: join(entities.path, groupPrefix) },
})
}
Expand Down
Loading