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

fix broken rule, set Angular/Typescript as optionalPeerDeps #32

Merged
merged 2 commits into from
Dec 18, 2024
Merged
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
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module.exports = {
root: true,
extends: ['eslint:recommended', 'plugin:eslint-plugin/recommended', 'plugin:node/recommended'],
extends: ['eslint:recommended', 'plugin:eslint-plugin/recommended', 'plugin:n/recommended'],
env: {
node: true,
},
Expand Down
18 changes: 13 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

Please see [the README](./README.md) for details of added rules.

## 5.4.0

- no-indexed-access-on-enums - fix missing import that caused eslint to crash when using this rule
- Mark Angular and Typescript peerDependencies as optional
- Switch from eslint-plugin-node to eslint-plugin-n
- eslint-plugin-node is no no longer maintained, eslint-plugin-n is a fork
- no-spreading-accumulators - pass error message in to remove eslint-plugin/prefer-message-ids lint error

## [5.3.1]

- Use `eslint-plugin-cypress` version below 4, as versions from 4.0.0 onward require ESLint v9, which is not yet supported by `eslint-plugin-criteo`.
Expand Down Expand Up @@ -38,9 +46,9 @@ Please see [the README](./README.md) for details of added rules.
### Changed

- Removal of the rules `deny-constructor-di` and `import-inject-object` from `@rdlabo/eslint-plugin-rules` because the auto-fix has too many issues:
- It applies on irrelevant places cf https://github.com/rdlabo-team/eslint-plugin-rules/issues/1#issuecomment-1980955010
- It gets lost with access modifiers cf https://github.com/rdlabo-team/eslint-plugin-rules/issues/4
- It generates broken code cf https://github.com/rdlabo-team/eslint-plugin-rules/issues/5
- It applies on irrelevant places cf <https://github.com/rdlabo-team/eslint-plugin-rules/issues/1#issuecomment-1980955010>
- It gets lost with access modifiers cf <https://github.com/rdlabo-team/eslint-plugin-rules/issues/4>
- It generates broken code cf <https://github.com/rdlabo-team/eslint-plugin-rules/issues/5>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were added by my local markdown linter. Looks like it's shorthand for a URL. https://www.markdownguide.org/basic-syntax/#urls-and-email-addresses
Github seems to render them correctly anyway, but it might as well be correct.

- Removal of deprecated configurations

## [5.1.0]
Expand All @@ -63,7 +71,7 @@ Please see [the README](./README.md) for details of added rules.

### Changed

- Rule `no-indexed-access-on-enums` disabled due to the bug https://github.com/criteo/eslint-plugin-criteo/issues/30
- Rule `no-indexed-access-on-enums` disabled due to the bug <https://github.com/criteo/eslint-plugin-criteo/issues/30>
- Rule `ngx-component-display` now ignores components matching `^.*(?:Dialog|Modal)Component$` (previously, only `^.*DialogComponent$` were ignored)

## [4.12.0]
Expand Down Expand Up @@ -169,7 +177,7 @@ Please see [the README](./README.md) for details of added rules.
### Changed

- [BREAKING] Upgrade Angular plugins to version 13 for compatibility with Angular 13
- [BREAKING] Move dependent plugins to peerDependencies so that they appear in the root node_modules (https://github.com/criteo/eslint-plugin-criteo/issues/15)
- [BREAKING] Move dependent plugins to peerDependencies so that they appear in the root node_modules (<https://github.com/criteo/eslint-plugin-criteo/issues/15>)
- [BREAKING] Update `engines` field in package.json to only allow npm versions >= 7

## [3.2.1]
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/filename-match-export.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
'use strict';

const path = require('path');
const path = require('node:path');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New recommended way to refer to builtin node packages.


const clean = (value) => (value ?? '').toLowerCase().replace(/[^a-z0-9]+/g, '');

Expand Down
2 changes: 1 addition & 1 deletion lib/rules/filename.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
'use strict';

const path = require('path');
const path = require('node:path');
const {
COMPONENT_SELECTOR,
STYLE_URL_SELECTOR,
Expand Down
6 changes: 3 additions & 3 deletions lib/rules/independent-folders.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
'use strict';

const path = require('path');
const path = require('node:path');

const isUnderPath = (folderPath, targetPath) => {
const relative = path.relative(folderPath, targetPath);
Expand Down Expand Up @@ -87,8 +87,8 @@ module.exports = {
buildReport(
node,
featureFolderIndex !== -1 ? featureFolders[featureFolderIndex] : sharedFolders[sharedFolderIndex],
otherFeatureFolders[featureFolderImportedIndex]
)
otherFeatureFolders[featureFolderImportedIndex],
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added and ran prettier

);
}
},
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/ngx-component-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ module.exports = {

// Try to get the HostBinding decorator
const decorator = (displayNode.decorators || []).find(
(decorator) => decorator.expression.callee.name === 'HostBinding'
(decorator) => decorator.expression.callee.name === 'HostBinding',
);

if (!decorator) {
Expand Down
1 change: 1 addition & 0 deletions lib/rules/no-indexed-access-on-enums.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
'use strict';

const ts = require('typescript');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This missing import caused this rule to error if it were ever used.

const moreInfo = `More info: https://github.com/criteo/eslint-plugin-criteo#no-indexed-access-on-enums`;

module.exports = {
Expand Down
19 changes: 10 additions & 9 deletions lib/rules/no-spreading-accumulators.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ module.exports = {
type: 'problem',
fixable: 'code',
schema: [],
messages: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

objectMessage: `Creating a new object by spreading the previous accumulator at every iteration of a .reduce() call has O(n^2) time & spatial complexity. If possible, make this O(n) by assigning to the existing accumulator or use mappedBy() or mappedByUnique() instead of .reduce(). ${moreInfo}`,
arrayMessage: `Creating a new array by spreading the previous accumulator at every iteration of a .reduce() call has O(n^2) time & spatial complexity. If possible, make this O(n) by pushing to the existing accumulator instead. ${moreInfo}`,
},
},
create: function (context) {
return {
'CallExpression:has([callee.property.name=reduce])[arguments] > ArrowFunctionExpression'(node) {
const objectMessage = `Creating a new object by spreading the previous accumulator at every iteration of a .reduce() call has O(n^2) time & spatial complexity. If possible, make this O(n) by assigning to the existing accumulator or use mappedBy() or mappedByUnique() instead of .reduce(). ${moreInfo}`;
const arrayMessage = `Creating a new array by spreading the previous accumulator at every iteration of a .reduce() call has O(n^2) time & spatial complexity. If possible, make this O(n) by pushing to the existing accumulator instead. ${moreInfo}`;

// only fetch the source code if getNodeText is called, and only fetch it once if we do
const sourceCode = {
_source: null,
Expand All @@ -29,10 +30,10 @@ module.exports = {
},
};

const getReportConfig = (spreadNode, fix, message) => ({
const getReportConfig = (spreadNode, fix, messageId) => ({
node: spreadNode.argument,
loc: spreadNode.loc,
message,
messageId,
fix,
});

Expand Down Expand Up @@ -83,7 +84,7 @@ module.exports = {
const nonSpreadPropertyTexts = expression.properties
.filter((o, i) => i != spreadIndex)
.map(
(o) => `${arrowFnFirstArgName}[${sourceCode.getNodeText(o.key)}] = ${sourceCode.getNodeText(o.value)}`
(o) => `${arrowFnFirstArgName}[${sourceCode.getNodeText(o.key)}] = ${sourceCode.getNodeText(o.value)}`,
);
if (nonSpreadPropertyTexts.length <= 0) return undefined;

Expand All @@ -92,7 +93,7 @@ module.exports = {
const fix = (fixer) => [fixer.replaceTextRange(arrowFn.range, newArrowFnText)];
return fix;
};
const reportConfig = getReportConfig(spreadNode, getObjectFix(), objectMessage);
const reportConfig = getReportConfig(spreadNode, getObjectFix(), 'objectMessage');
return context.report(reportConfig);
} else if (expression.type === 'ArrayExpression') {
let spreadIndex = null;
Expand Down Expand Up @@ -130,13 +131,13 @@ module.exports = {
if (!nonSpreadElementTexts || nonSpreadElementTexts.length <= 0) return undefined;

const bodyText = `${arrowFnFirstArgName}.push(${nonSpreadElementTexts.join(
', '
', ',
)}); return ${arrowFnFirstArgName};`;
const newArrowFnText = `(${paramsText}) => { ${bodyText} }`;
const fix = (fixer) => [fixer.replaceTextRange(arrowFn.range, newArrowFnText)];
return fix;
};
const reportConfig = getReportConfig(spreadNode, getArrayFix(), arrayMessage);
const reportConfig = getReportConfig(spreadNode, getArrayFix(), 'arrayMessage');
return context.report(reportConfig);
}
},
Expand Down
Loading