Skip to content

Commit

Permalink
Move getRuleDescriptionUrl to plugins (#151)
Browse files Browse the repository at this point in the history
Co-authored-by: Lev Chelyadinov <[email protected]>
  • Loading branch information
daniilsapa and illright authored Dec 24, 2024
1 parent cb661a9 commit a1b458c
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 65 deletions.
7 changes: 7 additions & 0 deletions .changeset/two-dragons-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@feature-sliced/steiger-plugin': minor
'steiger': minor
'@steiger/types': minor
---

Move getRuleDescriptionUrl from Steiger core to plugins
8 changes: 6 additions & 2 deletions packages/pretty-reporter/example/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ reportPretty(
severity: 'error',
getRuleDescriptionUrl,
},
{
message: 'Example of a diagnostic without a description URL',
ruleName: 'dummy/rule-name',
location: { path: '/home/user/project/src/pages' },
severity: 'error',
},
],
'/home/user/project',
)

// reportPretty([], '/home/user/project')
4 changes: 3 additions & 1 deletion packages/pretty-reporter/src/format-single-diagnostic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ export function formatSingleDiagnostic(d: Diagnostic, cwd: string): string {
const message = chalk.reset(d.message)
const autofixable = d.fixes !== undefined && d.fixes.length > 0 ? chalk.green(`${figures.tick} Auto-fixable`) : null
const location = chalk.gray(formatLocation(d.location, cwd))
const ruleName = chalk.blue(terminalLink(d.ruleName, d.getRuleDescriptionUrl(d.ruleName).toString()))
const ruleName = d.getRuleDescriptionUrl
? chalk.blue(terminalLink(d.ruleName, d.getRuleDescriptionUrl(d.ruleName).toString()))
: chalk.reset(d.ruleName)

return `
${s} ${location}
Expand Down
6 changes: 6 additions & 0 deletions packages/steiger-plugin-fsd/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import typoInLayerName from './typo-in-layer-name/index.js'
import noProcesses from './no-processes/index.js'
import packageJson from '../package.json'

const FSD_RULE_DESC_SOURCE = 'https://github.com/feature-sliced/steiger/tree/master/packages/steiger-plugin-fsd/src/'

const rules = [
ambiguousSliceNames,
excessiveSlicing,
Expand All @@ -44,6 +46,10 @@ const plugin = createPlugin({
name: '@feature-sliced/steiger-plugin',
version: packageJson.version,
},
getRuleDescriptionUrl(ruleName: string) {
const ruleNameWithoutNamespace = ruleName.split('/')[1]
return new URL(`${FSD_RULE_DESC_SOURCE}${ruleNameWithoutNamespace}`)
},
ruleDefinitions: rules,
})

Expand Down
33 changes: 18 additions & 15 deletions packages/steiger/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,11 @@ import type { Config, Folder, Rule } from '@steiger/types'

import { scan, createWatcher } from './features/transfer-fs-to-vfs'
import { defer } from './shared/defer'
import { $enabledRules, getEnabledRules, getGlobalIgnores } from './models/config'
import { $enabledRules, getEnabledRules, getGlobalIgnores, getPluginByRuleName } from './models/config'
import { runRule } from './features/run-rule'
import { removeGlobalIgnoreFromVfs } from './features/remove-global-ignores-from-vfs'
import { calculateFinalSeverities } from './features/calculate-diagnostic-severities'

// TODO: make this part of a plugin
function getRuleDescriptionUrl(ruleName: string) {
const withoutNamespace = ruleName.split('/')[1]
return new URL(
`https://github.com/feature-sliced/steiger/tree/master/packages/steiger-plugin-fsd/src/${withoutNamespace}`,
)
}

async function runRules({ vfs, rules }: { vfs: Folder; rules: Array<Rule> }) {
const vfsWithoutGlobalIgnores = removeGlobalIgnoreFromVfs(vfs, getGlobalIgnores())

Expand All @@ -28,18 +20,29 @@ async function runRules({ vfs, rules }: { vfs: Folder; rules: Array<Rule> }) {
}

const ruleName = rules[ruleResultsIndex].name
const ruleSourcePlugin = getPluginByRuleName(ruleName)
const severities = calculateFinalSeverities(
vfsWithoutGlobalIgnores,
ruleName,
diagnostics.map((d) => d.location.path),
)

return diagnostics.map((d, index) => ({
...d,
ruleName,
getRuleDescriptionUrl,
severity: severities[index],
}))
return diagnostics.map((d, index) => {
const finalDiagnostic = {
...d,
ruleName,
severity: severities[index],
}

if (ruleSourcePlugin.getRuleDescriptionUrl) {
return {
...finalDiagnostic,
getRuleDescriptionUrl: ruleSourcePlugin.getRuleDescriptionUrl,
}
}

return finalDiagnostic
})
})
}

Expand Down
11 changes: 11 additions & 0 deletions packages/steiger/src/models/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ export function getEnabledRules() {
return $enabledRules.getState()
}

export function getPluginByRuleName(ruleName: string) {
const sourcePlugin = $plugins
.getState()
.find((plugin) => plugin.ruleDefinitions.some((rule) => rule.name === ruleName))
if (sourcePlugin === undefined) {
throw new Error(`Expected to find a source plugin for the rule "${ruleName}", but didn't`)
}

return sourcePlugin
}

export function getRuleOptions(ruleName: string) {
return $ruleInstructions.getState()?.[ruleName].options || null
}
Expand Down
96 changes: 50 additions & 46 deletions packages/steiger/src/models/config/validate-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,32 @@ import { describe, expect, it } from 'vitest'
import { Config, Plugin, Rule } from '@steiger/types'

import { buildValidationScheme, validateConfig } from './validate-config'
import { isPlugin } from './raw-config'

// The function maps a plugin object to a new object with the same properties as the original object,
// but with the check method expected to be just any function.
// The reason why it's needed is that the original check method's .toString method returns [Function: check],
// but after the validation it changes to [Function: anonymous] and fails the test.
function expectCheckToBeAnyFunction(config: Config<Array<Rule>>) {
return config.map((configObject) => {
if (isPlugin(configObject)) {
return {
...configObject,
ruleDefinitions: configObject.ruleDefinitions.map((ruleDefinition) => ({
...ruleDefinition,
check: expect.any(Function),
})),
}
}
return configObject
})

const dummyPluginWithGetRuleDescriptionUrl: Plugin = {
meta: {
name: 'dummy',
version: '1.0.0',
},
getRuleDescriptionUrl(ruleName: string): URL {
return new URL(`http://example.com/${ruleName}`)
},
ruleDefinitions: [
{
name: 'rule1',
check() {
return { diagnostics: [] }
},
},
{
name: 'rule2',
check() {
return { diagnostics: [] }
},
},
],
}

const dummyPlugin: Plugin = {
const dummyPluginWithoutGetRuleDescriptionUrl: Plugin = {
meta: {
name: 'dummy',
version: '1.0.0',
Expand All @@ -47,7 +51,7 @@ const dummyPlugin: Plugin = {
describe('buildValidationScheme', () => {
it('should successfully validate config with plain severities', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
rules: {
rule1: 'off',
Expand All @@ -57,12 +61,12 @@ describe('buildValidationScheme', () => {
] as Config<Array<Rule>>
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should successfully validate config with a tuple of severity and rule options', () => {
const config = [
dummyPlugin,
dummyPluginWithoutGetRuleDescriptionUrl,
{
rules: {
rule1: ['warn', {}],
Expand All @@ -72,12 +76,12 @@ describe('buildValidationScheme', () => {
] as Config<Array<Rule>>
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should successfully validate a config with several objects', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
rules: {
rule1: 'off',
Expand All @@ -92,12 +96,12 @@ describe('buildValidationScheme', () => {

const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should successfully validate a config with ignores', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
ignores: ['/shared'],
rules: {
Expand All @@ -113,12 +117,12 @@ describe('buildValidationScheme', () => {
] as Config<Array<Rule>>
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should successfully validate a config with files', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,

{
files: ['/shared'],
Expand All @@ -135,12 +139,12 @@ describe('buildValidationScheme', () => {
] as Config<Array<Rule>>
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should successfully validate a config with files and ignores', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
files: ['/shared'],
ignores: ['**/*.test.ts'],
Expand All @@ -158,7 +162,7 @@ describe('buildValidationScheme', () => {
] as Config<Array<Rule>>
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should throw an error if no plugins are provided', () => {
Expand All @@ -173,16 +177,16 @@ describe('buildValidationScheme', () => {
})

it('should throw an error if no config objects are provided', () => {
const config = [dummyPlugin] as Config<Array<Rule>>
const config = [dummyPluginWithGetRuleDescriptionUrl] as Config<Array<Rule>>
const scheme = buildValidationScheme(config)

expect(() => scheme.parse(config)).toThrow('At least one config object must be provided!')
})

it('should throw an error if a config object without rules is provided', () => {
const config = [dummyPlugin, {}] as Config<Array<Rule>>
const config1 = [dummyPlugin, { files: ['/shared'] }] as Config<Array<Rule>>
const config2 = [dummyPlugin, { ignores: ['**/*.test.ts'] }] as Config<Array<Rule>>
const config = [dummyPluginWithGetRuleDescriptionUrl, {}] as Config<Array<Rule>>
const config1 = [dummyPluginWithGetRuleDescriptionUrl, { files: ['/shared'] }] as Config<Array<Rule>>
const config2 = [dummyPluginWithGetRuleDescriptionUrl, { ignores: ['**/*.test.ts'] }] as Config<Array<Rule>>

const scheme = buildValidationScheme(config)
const scheme1 = buildValidationScheme(config1)
Expand All @@ -195,7 +199,7 @@ describe('buildValidationScheme', () => {

it('should throw an error if a non-existent rule is provided', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
rules: {
rule1: 'off',
Expand All @@ -210,7 +214,7 @@ describe('buildValidationScheme', () => {

it('should correctly validate a config with global ignores', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
ignores: ['/src/shared/**'],
},
Expand All @@ -223,12 +227,12 @@ describe('buildValidationScheme', () => {
]
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should correctly validate a config with unique rule options', () => {
const config = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
ignores: ['/src/shared/**'],
},
Expand All @@ -241,12 +245,12 @@ describe('buildValidationScheme', () => {
]
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should successfully validate when the config provides a rule with multiple but identical options', () => {
const config: Config<Array<Rule>> = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
rules: {
rule1: ['warn', { option1: 'value1' }],
Expand All @@ -262,12 +266,12 @@ describe('buildValidationScheme', () => {
]
const scheme = buildValidationScheme(config)

expect(scheme.parse(config)).toEqual(expectCheckToBeAnyFunction(config))
expect(() => scheme.parse(config)).not.toThrow()
})

it('should throw an error when the config provides a rule with multiple but different options', () => {
const config: Config<Array<Rule>> = [
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
{
rules: {
rule1: ['warn', { option1: 'value1' }],
Expand All @@ -293,8 +297,8 @@ describe('buildValidationScheme', () => {

it('should throw an error when duplicate rule definition are provided', () => {
const config: Config<Array<Rule>> = [
dummyPlugin,
dummyPlugin,
dummyPluginWithGetRuleDescriptionUrl,
dummyPluginWithGetRuleDescriptionUrl,
{
rules: {
rule1: ['warn', { option1: 'value1' }],
Expand Down
1 change: 1 addition & 0 deletions packages/steiger/src/models/config/validate-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export function buildValidationScheme(rawConfig: Config<Array<Rule>>) {
name: z.string(),
version: z.string(),
}),
getRuleDescriptionUrl: z.optional(z.function().args(z.string()).returns(z.any())),
ruleDefinitions: z.array(
z.object({
name: z.string(),
Expand Down
3 changes: 2 additions & 1 deletion packages/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface PartialDiagnostic {
export interface Diagnostic extends PartialDiagnostic {
ruleName: string
severity: Exclude<Severity, 'off'>
getRuleDescriptionUrl(ruleName: string): URL
getRuleDescriptionUrl?(ruleName: string): URL
}

export type Fix =
Expand Down Expand Up @@ -112,5 +112,6 @@ export interface Plugin<Context = unknown, Rules extends Array<Rule<Context>> =
name: string
version: string
}
getRuleDescriptionUrl?(ruleName: string): URL
ruleDefinitions: Rules
}

0 comments on commit a1b458c

Please sign in to comment.