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

Jump to bottom

[DO NOT REVIEW] Migrate to eslint 9 and flat config #2192

Draft
wants to merge 123 commits into
base: main
Choose a base branch
from
Draft

[DO NOT REVIEW] Migrate to eslint 9 and flat config #2192

wants to merge 123 commits into from

Conversation

matthieu-crouzet
Copy link
Contributor

@matthieu-crouzet matthieu-crouzet commented Sep 23, 2024 •
edited
Loading

Proposed change

Migrate to eslint 9 and flat config

Missing in this PR:

  • Review with the team the config exposed in @o3r/eslint-config
  • Fix errors due to the new config
  • Documentation:
    • Difference of rules between the 2 configs
    • Update global documentation (docs/linter/*)
  • fix ng-add @o3r/eslint-config
  • fix jsdoc/check-examples
  • Bonus: CLI to detect deprecated rules

@matthieu-crouzet matthieu-crouzet changed the title Migrate to eslint 9 and flat config [DO NOT REVIEW] Migrate to eslint 9 and flat config Sep 23, 2024
@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Sep 23, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cd54143. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

🟥 Failed Commands
nx run-many --target=test-int --parallel 3
nx affected --target=lint --parallel 3 --base=remotes/origin/main --configuration ci
✅ Successfully ran 9 targets

Sent with 💌 from NxCloud.

matthieu-crouzet and others added 11 commits November 7, 2024 15:03
@matthieu-crouzet
Merge branch 'main' of https://github.com/AmadeusITGroup/otter into e…
67119ae 
…slint-merge-main-ter
@matthieu-crouzet
Eslint merge main (#2423)
076ef25
@matthieu-crouzet
fix(eslint): fix @o3r-training packages
d5f87f2
@matthieu-crouzet
fix(eslint): fix @o3r-training packages (#2424)
fa913a6
@matthieu-crouzet
fix(eslint): fix process-exit rule (#2422)
1416271
@matthieu-crouzet
fix(eslint): fix remaining rules after team discussion
59694d8
@matthieu-crouzet
fix(eslint): fix remaining rules after team discussion (#2426)
f430b39
@matthieu-crouzet
docs(eslint): stylistic difference with eslintrc
a651437
@matthieu-crouzet
Merge branch 'main' of https://github.com/AmadeusITGroup/otter into HEAD
8e2f59f
@matthieu-crouzet
docs(eslint): stylistic difference with eslintrc (#2432)
b92003f 
## Proposed change

<!--
Please include a summary of the changes and the related issue.
Please also include relevant motivation and context.
-->

## Related issues

<!--
Please make sure to follow the [contribution
guidelines](https://github.com/amadeus-digital/Otter/blob/main/CONTRIBUTING.md)
-->

*- No issue associated -*

<!-- * 🐛 Fix #issue -->
<!-- * 🐛 Fix resolves #issue -->
<!-- * 🚀 Feature #issue -->
<!-- * 🚀 Feature resolves #issue -->
<!-- * :octocat: Pull Request #issue -->
@matthieu-crouzet
fix(eslint): fix remaining comma after rebase
bb11321
@matthieu-crouzet matthieu-crouzet mentioned this pull request Nov 15, 2024
Open
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 22, 2024
View reviewed changes
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
@@ -42,27 +52,27 @@
*/
private parseCssVariable(name: string, value = ''): CssVariable {
const defaultValue = value.trim();
const res = defaultValue.match(/^var\( *([^,)]*) *(?:, *([^,()]*(\(.*\))?))*\)$/);
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with 'var(,' and containing many repetitions of ' ,'.

Copilot Autofix AI 1 day ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that causes exponential backtracking. This can be achieved by making the sub-expressions more specific and avoiding nested quantifiers.

  • The original regular expression on line 55 is: ^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$
  • We will modify it to: ^var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\(.*\))?))*\s*\)$

This change ensures that the sub-expressions are more specific and reduces the risk of catastrophic backtracking.

Suggested changeset 1
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
--- a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
+++ b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
@@ -54,3 +54,3 @@
     const defaultValue = value.trim();
-    const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
+    const res = defaultValue.match(/^var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\(.*\))?))*\s*\)$/);
     const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
       do {
-        ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
+        ref = /var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\(.*\))?))*\s*\)/.exec(findRef);
         if (ref) {
EOF
@@ -54,3 +54,3 @@
const defaultValue = value.trim();
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
const res = defaultValue.match(/^var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\(.*\))?))*\s*\)$/);
const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
do {
ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
ref = /var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\(.*\))?))*\s*\)/.exec(findRef);
if (ref) {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
@@ -42,27 +52,27 @@
*/
private parseCssVariable(name: string, value = ''): CssVariable {
const defaultValue = value.trim();
const res = defaultValue.match(/^var\( *([^,)]*) *(?:, *([^,()]*(\(.*\))?))*\)$/);
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with 'var(,(' and containing many repetitions of '),('.

Copilot Autofix AI 1 day ago

To fix the problem, we need to modify the regular expression to avoid the use of ambiguous patterns that can cause exponential backtracking. Specifically, we should replace the .* pattern with a more precise pattern that matches the expected characters without causing ambiguity.

  • The original regular expression on line 55 is: ^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$
  • We will replace .* with a more specific pattern that matches any character except for parentheses, commas, and spaces, which are the delimiters in this context.
Suggested changeset 1
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
--- a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
+++ b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
@@ -54,3 +54,3 @@
     const defaultValue = value.trim();
-    const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
+    const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*\([^()]*\))?)*\)$/);
     const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
       do {
-        ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
+        ref = /var\( *([^),]*) *(?:, *([^(),]*\([^()]*\))?)*\)/.exec(findRef);
         if (ref) {
EOF
@@ -54,3 +54,3 @@
const defaultValue = value.trim();
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*\([^()]*\))?)*\)$/);
const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
do {
ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
ref = /var\( *([^),]*) *(?:, *([^(),]*\([^()]*\))?)*\)/.exec(findRef);
if (ref) {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
let findRef = defaultValue;
let ref: RegExpExecArray | null;
const references: Record<string, CssVariable> = {};
do {
ref = /var\( *([^,)]*) *(?:, *([^,()]*(\(.*\))?))*\)/.exec(findRef);
ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with 'var(,' and containing many repetitions of ' ,'.

Copilot Autofix AI 1 day ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that causes exponential backtracking. This can be achieved by making the sub-expressions more specific and avoiding nested quantifiers that can match the same string in multiple ways.

The best way to fix the problem is to rewrite the regular expression to ensure that each part of the pattern is unambiguous. Specifically, we can replace the ambiguous parts with more precise patterns that do not overlap in their matching capabilities.

Suggested changeset 1
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
--- a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
+++ b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
@@ -54,3 +54,3 @@
     const defaultValue = value.trim();
-    const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
+    const res = defaultValue.match(/^var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\([^)]*\))?))*\s*\)$/);
     const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
       do {
-        ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
+        ref = /var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\([^)]*\))?))*\s*\)/.exec(findRef);
         if (ref) {
EOF
@@ -54,3 +54,3 @@
const defaultValue = value.trim();
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
const res = defaultValue.match(/^var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\([^)]*\))?))*\s*\)$/);
const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
do {
ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
ref = /var\(\s*([^),\s]+)\s*(?:,\s*([^(),\s]+(?:\([^)]*\))?))*\s*\)/.exec(findRef);
if (ref) {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
let findRef = defaultValue;
let ref: RegExpExecArray | null;
const references: Record<string, CssVariable> = {};
do {
ref = /var\( *([^,)]*) *(?:, *([^,()]*(\(.*\))?))*\)/.exec(findRef);
ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with 'var(,(' and containing many repetitions of '),('.

Copilot Autofix AI 1 day ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that causes exponential backtracking. Specifically, we should replace the .* pattern with a more specific pattern that avoids matching in multiple ways. In this case, we can replace .* with [^()]* to ensure that the pattern matches any character except parentheses, which are the delimiters in the context of this regular expression.

  • Modify the regular expression on line 55 and line 66 to replace .* with [^()]*.
  • Ensure that the new pattern maintains the existing functionality of matching CSS variable references.
Suggested changeset 1
packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
--- a/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
+++ b/packages/@o3r/styling/builders/style-extractor/helpers/css-variable.extractor.ts
@@ -54,3 +54,3 @@
     const defaultValue = value.trim();
-    const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
+    const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\([^()]*\))?))*\)$/);
     const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
       do {
-        ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
+        ref = /var\( *([^),]*) *(?:, *([^(),]*(\([^()]*\))?))*\)/.exec(findRef);
         if (ref) {
EOF
@@ -54,3 +54,3 @@
const defaultValue = value.trim();
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)$/);
const res = defaultValue.match(/^var\( *([^),]*) *(?:, *([^(),]*(\([^()]*\))?))*\)$/);
const ret: CssVariable = { name, defaultValue };
@@ -65,3 +65,3 @@
do {
ref = /var\( *([^),]*) *(?:, *([^(),]*(\(.*\))?))*\)/.exec(findRef);
ref = /var\( *([^),]*) *(?:, *([^(),]*(\([^()]*\))?))*\)/.exec(findRef);
if (ref) {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@codecov Codecov
Copy link

codecov bot commented Nov 22, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 91.66667% with 20 lines in your changes missing coverage. Please review.

Project coverage is 65.84%. Comparing base (4c351b1) to head (cd54143).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../src/components/showcase/sdk/sdk-pres.component.ts 62.50% 6 Missing ⚠️
...owcase/design-token/design-token-pres.component.ts 70.00% 3 Missing ⚠️
...ps/github-cascading-app/src/cascading/cascading.ts 80.00% 2 Missing ⚠️
.../operators/during-summer/during-summer.operator.ts 33.33% 2 Missing ⚠️
.../client-fetch/src/plugins/timeout/timeout.fetch.ts 71.42% 0 Missing and 2 partials ⚠️
...e/src/app/configuration/configuration.component.ts 85.71% 1 Missing ⚠️
.../scroll-back-top/scroll-back-top-pres.component.ts 83.33% 1 Missing ⚠️
...t-fetch/src/plugins/keepalive/keepalive.request.ts 50.00% 1 Missing ⚠️
...src/plugins/mock-intercept/mock-intercept.fetch.ts 66.66% 0 Missing and 1 partial ⚠️
...fetch/src/plugins/perf-metric/perf-metric.fetch.ts 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

@matthieu-crouzet matthieu-crouzet linked an issue Nov 22, 2024 that may be closed by this pull request
[Feature]: Review and cleanup eslint-config #2052
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working enhancement New feature or request project:@ama-sdk/core project:@ama-sdk/create project:@ama-sdk/schematics project:@ama-sdk/showcase-sdk project:@ama-sdk/swagger-builder project:@ama-terasu/cli project:@ama-terasu/core project:@ama-terasu/schematics project:audit-gh-action project:cascading-gh-action project:get-npm-tag-gh-action project:new-version-gh-action project:@o3r/amaterasu-api-spec project:@o3r/amaterasu-dodo project:@o3r/amaterasu-otter project:@o3r/amaterasu-sdk project:@o3r/analytics project:@o3r/apis-manager project:@o3r/application project:@o3r/artifactory-tools project:@o3r/azure-tools project:@o3r/build-helpers project:@o3r/chrome-devtools project:@o3r/components project:@o3r/configuration project:@o3r/core project:@o3r/create project:@o3r/design project:@o3r/dev-tools project:@o3r/dynamic-content project:@o3r/eslint-config project:@o3r/eslint-config-otter project:@o3r/eslint-plugin project:@o3r/extractors project:@o3r/forms project:@o3r/github-cascading-app project:@o3r/localization project:@o3r/logger project:@o3r/mobile project:@o3r/new-version project:@o3r/routing project:@o3r/rules-engine project:@o3r/schematics project:@o3r/showcase project:@o3r/store-sync project:@o3r/storybook project:@o3r/stylelint-plugin project:@o3r/styling project:@o3r/telemetry project:@o3r/test-helpers project:@o3r/testing project:@o3r/third-party project:@o3r/workspace project:otter-devtools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature]: Review and cleanup eslint-config
4 participants
@matthieu-crouzet @kpanot @sdo-1A @fpaul-1A