chore(eslint): enable rules globally and fix violations#7548
Conversation
The following rules now apply to all files (enabled in global config): - `eslint-rules/eslint-safe-typeof-object` - `no-implicit-coercion` - `no-useless-assignment` - `no-void` - `operator-assignment` - `prefer-exponentiation-operator` - `prefer-object-has-own` - `n/no-restricted-require` (using shared `GLOBAL_RESTRICTED_REQUIRES`) Removed these from the `dd-trace/src/all` block to avoid duplication. Fix tests and app code for the new global rules: - Safe typeof checks - Remove useless assignments - Use `**` and template literals - Use `filter.includes()` instead of `~indexOf`) - Fix typo in vertexai test (`} if` → `} else if`).
Overall package sizeSelf size: 4.62 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.6 | 81.92 kB | 813.08 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7548 +/- ##
===========================================
+ Coverage 59.58% 80.15% +20.57%
===========================================
Files 425 730 +305
Lines 18259 31217 +12958
===========================================
+ Hits 10880 25023 +14143
+ Misses 7379 6194 -1185 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| vulnerabilitiesTrace.vulnerabilities.forEach(v => { | ||
| let count = vulnerabilitiesCount.get(v.type) || 0 | ||
| vulnerabilitiesCount.set(v.type, ++count) | ||
| const count = (vulnerabilitiesCount.get(v.type) || 0) + 1 |
There was a problem hiding this comment.
| const count = (vulnerabilitiesCount.get(v.type) || 0) + 1 | |
| const count = vulnerabilitiesCount.get(v.type) + 1 || 1 |
There was a problem hiding this comment.
That's not identical. If vulnerabilitiesCount.get(v.type) is undefined we need the fallback to 0.
There was a problem hiding this comment.
Before suggesting I of course double checked it:
> (undefined || 0) + 1
1
> undefined + 1 || 1
1There was a problem hiding this comment.
That's only because NaN is falsy. I don't think relying on that is such a good idea. Our TS checker would definitely not allow it.
There was a problem hiding this comment.
It is also more difficult to process in my opinion. We have to know that the OR is of higher precedence and that this works due to being coerced to NaN which is falsy.
The brackets are straight forward to understand.
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
LGTM, just left two suggestions
| for (let i = 0; i < 5; i++) { | ||
| nowValue += 20 | ||
| const event = { startTime: 0, duration: filter.nextSamplingInstant - 0 } | ||
| const event = { startTime: 0, duration: filter.nextSamplingInstant } |
There was a problem hiding this comment.
would - 0 be a way to coalesce the value to a number ? sounds improbable that someone actually wrote - 0 without a reason no ?
There was a problem hiding this comment.
Yes, I had the same thought and looked over the code to see if nextSamplingInstant would ever not be a number. As far as I can see, nextSamplingInstant is ALWAYS a number.
There was a problem hiding this comment.
... and the tests pass, so 👍
There was a problem hiding this comment.
This is in a test case, so if there is an issue with the test data, it would surface here. Neither before, nor after would it detect actual faulty user input that could theoretically cause issues.
Since the test passes, the simplification seems good.
* chore(eslint): enable rules globally and fix violations The following rules now apply to all files (enabled in global config): - `eslint-rules/eslint-safe-typeof-object` - `no-implicit-coercion` - `no-useless-assignment` - `no-void` - `operator-assignment` - `prefer-exponentiation-operator` - `prefer-object-has-own` - `n/no-restricted-require` (using shared `GLOBAL_RESTRICTED_REQUIRES`) Removed these from the `dd-trace/src/all` block to avoid duplication. Fix tests and app code for the new global rules: - Safe typeof checks - Remove useless assignments - Use `**` and template literals - Use `filter.includes()` instead of `~indexOf`) - Fix typo in vertexai test (`} if` → `} else if`).
* chore(eslint): enable rules globally and fix violations The following rules now apply to all files (enabled in global config): - `eslint-rules/eslint-safe-typeof-object` - `no-implicit-coercion` - `no-useless-assignment` - `no-void` - `operator-assignment` - `prefer-exponentiation-operator` - `prefer-object-has-own` - `n/no-restricted-require` (using shared `GLOBAL_RESTRICTED_REQUIRES`) Removed these from the `dd-trace/src/all` block to avoid duplication. Fix tests and app code for the new global rules: - Safe typeof checks - Remove useless assignments - Use `**` and template literals - Use `filter.includes()` instead of `~indexOf`) - Fix typo in vertexai test (`} if` → `} else if`).
3 participants
What does this PR do?
Enables several ESLint rules in the global config so they apply to all files (not only
srcdirs), and fixes existing violations across the repo. The sharedGLOBAL_RESTRICTED_REQUIRESlist is extracted and reused so restricted-require config stays in one place.Rules now applied globally:
eslint-rules/eslint-safe-typeof-objectno-implicit-coercionno-useless-assignmentno-voidoperator-assignmentprefer-exponentiation-operatorprefer-object-has-ownn/no-restricted-require(usingGLOBAL_RESTRICTED_REQUIRES)Those rules are removed from the
dd-trace/src/allblock to avoid duplication (exceptn/no-restricted-requirewhich addssemveras a restricted require tosrconly).Fix tests and app code for the new global rules:
**and template literalsfilter.includes()instead of~indexOf} if→} else if)Motivation
Unify lint rules across the codebase so test and non-src files follow the same style and safety rules.