Skip to content

Commit

Permalink
feat(config): linter to detect invalid config property type (#1089)
Browse files Browse the repository at this point in the history
## Proposed change
Linter to detect invalid config property type.
Every property in
[StrictConfiguration](https://github.com/AmadeusITGroup/otter/blob/main/packages/%40o3r/core/src/core/interfaces/configuration.ts#L27)
can be for example `string` or `number` but not the both at the same
time.
It cannot be check with TypeScript, so I propose a linter for that.
  • Loading branch information
matthieu-crouzet authored Nov 27, 2023
2 parents 8c42a9b + b17e3e4 commit 696113d
Showing 6 changed files with 255 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# @o3r/no-multiple-type-configuration-property

Ensures that the configuration property does not accept multiple types.

## How to use

```json
{
"@o3r/no-multiple-type-configuration-property": [
"error",
{
"supportedInterfaceNames": ["NestedConfiguration", "Configuration", "CustomConfigurationInterface"]
}
]
}
```

## Valid code example

```typescript
export interface MyFirstConfig extends Configuration {
myProp: string;
}
```

## Invalid code example

```typescript
export interface MyConfig extends Configuration {
myProp: string | number;
}
```
6 changes: 5 additions & 1 deletion packages/@o3r/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ import noInnerHTML from './rules/template/no-inner-html/no-inner-html';
import templateAsyncNumberLimitation from './rules/template/template-async-number-limitation/template-async-number-limitation';
import jsonDependencyVersionsHarmonize from './rules/json/json-dependency-versions-harmonize/json-dependency-versions-harmonize';
import matchingConfigurationName from './rules/typescript/matching-configuration-name/matching-configuration-name';
import noMultipleTypeConfigurationProperty from './rules/typescript/no-multiple-type-configuration-property/no-multiple-type-configuration-property';

module.exports = {
rules: {
@@ -13,17 +14,20 @@ module.exports = {
'template-async-number-limitation': templateAsyncNumberLimitation,
'o3r-widget-tags': o3rWidgetTags,
'json-dependency-versions-harmonize': jsonDependencyVersionsHarmonize,
'matching-configuration-name': matchingConfigurationName
'matching-configuration-name': matchingConfigurationName,
'no-multiple-type-configuration-property': noMultipleTypeConfigurationProperty
},
configs: {
'@o3r/no-folder-import-for-module': 'error',
'@o3r/json-dependency-versions-harmonize': 'error',
'@o3r/no-multiple-type-configuration-property': 'error',
'@o3r/template-async-number-limitation': 'warn',
'@o3r/matching-configuration-name': 'warn',

recommended: {
rules: {
'@o3r/matching-configuration-name': 'error',
'@o3r/no-multiple-type-configuration-property': 'error',
'@o3r/no-folder-import-for-module': 'error',
'@o3r/template-async-number-limitation': 'off'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import noMultipleTypeConfigurationPropertyRule from './no-multiple-type-configuration-property';

const ruleTester = new TSESLint.RuleTester({
parser: require.resolve('@typescript-eslint/parser'),
parserOptions: {
ecmaVersion: 2018,
sourceType: 'module'
}
});

const code = `
export interface Config extends Configuration {
prop1: string;
prop2: 'a' | 'b' | 'c';
prop3: 1 | 2 | 3;
}
`;

ruleTester.run('no-multiple-type-configuration-property', noMultipleTypeConfigurationPropertyRule, {
valid: [
{ code },
{ code: 'export interface A { prop1: string | number; }'}
],
invalid: [
{
code: code.replace(': string;', ': string | number | boolean;'),
errors: [
{
suggestions: [
{
output: code,
messageId: 'suggestion',
data: {
currentValue: 'string | number | boolean',
recommendedValue: 'string'
}
},
{
output: code.replace(': string;', ': number;'),
messageId: 'suggestion',
data: {
currentValue: 'string | number | boolean',
recommendedValue: 'number'
}
},
{
output: code.replace(': string;', ': boolean;'),
messageId: 'suggestion',
data: {
currentValue: 'string | number | boolean',
recommendedValue: 'boolean'
}
}
],
messageId: 'error'
}
]
},
{
code: code.replace(': string;', ': string & number;'),
errors: [
{
suggestions: [
{
output: code,
messageId: 'suggestion',
data: {
currentValue: 'string & number',
recommendedValue: 'string'
}
},
{
output: code.replace(': string;', ': number;'),
messageId: 'suggestion',
data: {
currentValue: 'string & number',
recommendedValue: 'number'
}
}
],
messageId: 'error'
}
]
},
{
code: code.replace(': string;', ': \'a\' | 1;'),
errors: [
{
suggestions: [
{
output: code.replace(': string;', ': \'a\';'),
messageId: 'suggestion',
data: {
currentValue: '\'a\' | 1',
recommendedValue: '\'a\''
}
},
{
output: code.replace(': string;', ': 1;'),
messageId: 'suggestion',
data: {
currentValue: '\'a\' | 1',
recommendedValue: '1'
}
}
],
messageId: 'error'
}
]
}
]
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { TSESTree } from '@typescript-eslint/experimental-utils';
import { createRule, defaultSupportedInterfaceNames, isExtendingConfiguration } from '../../utils';

const separatorRegExp = /\s*[|&]\s*/;

export interface NoMultipleTypeConfigurationPropertyOption {
supportedInterfaceNames?: string[];
}

export default createRule<NoMultipleTypeConfigurationPropertyOption[], 'error' | 'suggestion'>({
name: 'no-multiple-type-configuration-property',
meta: {
hasSuggestions: true,
type: 'problem',
docs: {
description: 'Ensures that the configuration property does not accept multiple types.',
recommended: 'error'
},
schema: [
{
type: 'object',
properties: {
supportedInterfaceNames: {
type: 'array',
items: {
type: 'string'
},
default: defaultSupportedInterfaceNames
}
}
}
],
messages: {
error: 'Configuration cannot be the union of multiple type',
suggestion: 'Replace {{currentValue}} by {{recommendedValue}}'
}
},
defaultOptions: [],
create: (context) => {
const supportedInterfaceNames = context.options.reduce((acc: string[], option) => acc.concat(option.supportedInterfaceNames || []), []);
const sourceCode = context.getSourceCode();

const rule = (node: TSESTree.TSUnionType | TSESTree.TSIntersectionType) => {
const interfaceDeclNode = node.parent?.parent?.parent?.parent;
if (!isExtendingConfiguration(interfaceDeclNode, supportedInterfaceNames)) {
return; // Not in a configuration interface
}

if (
node.types.every((type) => type.type === TSESTree.AST_NODE_TYPES.TSLiteralType && type.literal.type === TSESTree.AST_NODE_TYPES.Literal)
&& [...(new Set((node.types as TSESTree.TSLiteralType[]).map((literalType) => typeof (literalType.literal as TSESTree.Literal).value)))].length === 1
) {
return; // Only the same literal type
}

const text = sourceCode.getText(node);
context.report({
messageId: 'error',
node,
loc: node.loc,
suggest: text.split(separatorRegExp).map((type) => ({
messageId: 'suggestion',
data: {
currentValue: text,
recommendedValue: type
},
fix: (fixer) => fixer.replaceTextRange(node.range, type)
}))
});
};

return {
// eslint-disable-next-line @typescript-eslint/naming-convention
TSUnionType: rule,
// eslint-disable-next-line @typescript-eslint/naming-convention
TSIntersectionType: rule
};
}
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
import { createRule } from '../../utils';
import { TSESLint } from '@typescript-eslint/experimental-utils';
import { createRule, defaultSupportedInterfaceNames, isExtendingConfiguration } from '../../utils';

const o3rWidgetParameterPattern = '^[a-zA-Z0-9-_:.]+$';

@@ -56,7 +56,8 @@ export default createRule<O3rWidgetTagsRuleOption[], O3rWidgetRuleErrorId>({
type: 'array',
items: {
type: 'string'
}
},
default: defaultSupportedInterfaceNames
},
widgets: {
additionalProperties: {
@@ -108,7 +109,7 @@ export default createRule<O3rWidgetTagsRuleOption[], O3rWidgetRuleErrorId>({
...option.widgets
};
return acc;
}, { widgets: {}, supportedInterfaceNames: ['Configuration', 'NestedConfiguration'] });
}, { widgets: {}, supportedInterfaceNames: [] });
const supportedO3rWidgets = new Set(Object.keys(options.widgets));
return {
// eslint-disable-next-line @typescript-eslint/naming-convention
@@ -165,14 +166,7 @@ export default createRule<O3rWidgetTagsRuleOption[], O3rWidgetRuleErrorId>({
}

const interfaceDeclNode = node.parent?.parent;
if (
interfaceDeclNode?.type !== TSESTree.AST_NODE_TYPES.TSInterfaceDeclaration
|| !interfaceDeclNode.extends?.some((ext) =>
ext.type === TSESTree.AST_NODE_TYPES.TSInterfaceHeritage
&& ext.expression.type === TSESTree.AST_NODE_TYPES.Identifier
&& options.supportedInterfaceNames.includes(ext.expression.name)
)
) {
if (!isExtendingConfiguration(interfaceDeclNode, options.supportedInterfaceNames)) {
return context.report({
messageId: 'notInConfigurationInterface',
node,
21 changes: 20 additions & 1 deletion packages/@o3r/eslint-plugin/src/rules/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ESLintUtils } from '@typescript-eslint/experimental-utils';
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import * as path from 'node:path';

/** Current package version (format: <major>.<minor>)*/
@@ -12,3 +12,22 @@ export const createRule = ESLintUtils.RuleCreator((name) => {
}
return `https://github.com/AmadeusITGroup/otter/tree/release/${version}/docs/linter/eslint-plugin/rules/${name}.md`;
});

/** Default supported interface names */
export const defaultSupportedInterfaceNames = ['Configuration', 'NestedConfiguration'];

/**
* Returns true if the node extends one of the `supportedInterfaceNames`
* @param interfaceDeclNode
* @param supportedInterfaceNames
*/
export const isExtendingConfiguration = (interfaceDeclNode: TSESTree.Node | undefined, supportedInterfaceNames: string[] = []) => {
const supportedInterfaceNamesSet = new Set(supportedInterfaceNames.length > 0 ? supportedInterfaceNames : defaultSupportedInterfaceNames);
return interfaceDeclNode?.type === TSESTree.AST_NODE_TYPES.TSInterfaceDeclaration
&& interfaceDeclNode.extends?.some((ext) =>
ext.type === TSESTree.AST_NODE_TYPES.TSInterfaceHeritage
&& ext.expression.type === TSESTree.AST_NODE_TYPES.Identifier
&& supportedInterfaceNamesSet.has(ext.expression.name)
);
};

0 comments on commit 696113d

Please sign in to comment.