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

upgrade eslint, eslint-plugin-n, eslint-config-standard (VULN-5530) #4032

Closed
wants to merge 10 commits into from
Closed
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 .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ node_modules
versions
acmeair-nodejs
vendor
integration-tests/esbuild/out.js
integration-tests/esbuild/*out*.js
40 changes: 33 additions & 7 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,50 @@
"es2020": true
},
"rules": {
"array-bracket-spacing": 0,
"dot-notation": 0,
"handle-callback-err": 0,
"max-len": [2, 120, 2],
"no-var": 2,
"multiline-ternary": 0,
"no-console": 2,
"prefer-const": 2,
"no-prototype-builtins": 0,
"no-var": 2,
"no-void": 0,
"object-curly-newline": ["error", {"multiline": true, "consistent": true }],
"object-curly-spacing": [2, "always"],
"object-shorthand": 0,
"prefer-const": 2,
"prefer-regex-literals": 0,
"quote-props": 0,
"quotes": 0,

"import/no-extraneous-dependencies": 2,
"standard/no-callback-literal": 0,
"no-prototype-builtins": 0,

"mocha/max-top-level-suites": 0,
"mocha/no-exports": 0,
"mocha/no-global-tests": 0,
"mocha/no-identical-title": 0,
"mocha/no-mocha-arrows": 0,
"mocha/no-setup-in-describe": 0,
"mocha/no-sibling-hooks": 0,
"mocha/no-skipped-tests": 0,
"mocha/no-top-level-hooks": 0,

"mocha/max-top-level-suites": 0,
"mocha/no-identical-title": 0,
"mocha/no-global-tests": 0,
"mocha/no-exports": 0,
"mocha/no-global-tests": 0,
"mocha/no-identical-title": 0,
"mocha/no-mocha-arrows": 0,
"mocha/no-setup-in-describe": 0,
"mocha/no-sibling-hooks": 0,
"mocha/no-skipped-tests": 0,
"mocha/no-top-level-hooks": 0,
"n/handle-callback-err": 0,
"n/no-callback-literal": 0,
"n/no-deprecated-api": 0,
"n/no-path-concat": 0,
"n/no-restricted-require": [2, ["diagnostics_channel"]],
"object-curly-newline": ["error", {"multiline": true, "consistent": true }]

"standard/no-callback-literal": 0
}
}
1 change: 0 additions & 1 deletion LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ dev,eslint-config-standard,MIT,Copyright Feross Aboukhadijeh
dev,eslint-plugin-import,MIT,Copyright 2015 Ben Mosher
dev,eslint-plugin-mocha,MIT,Copyright 2014 Mathias Schreck
dev,eslint-plugin-n,MIT,Copyright 2015 Toru Nagashima
dev,eslint-plugin-node,MIT,Copyright 2015 Toru Nagashima
dev,eslint-plugin-promise,ISC,jden and other contributors
dev,eslint-plugin-standard,MIT,Copyright 2015 Jamund Ferguson
dev,express,MIT,Copyright 2009-2014 TJ Holowaychuk 2013-2014 Roman Shtylman 2014-2015 Douglas Christopher Wilson
Expand Down
1 change: 0 additions & 1 deletion benchmark/e2e/express-helloworld-manyroutes/app.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const crypto = require('crypto')
const app = require('express')()

Expand Down
1 change: 1 addition & 0 deletions benchmark/sirun/appsec-iast/insecure-bank.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const http = require('http')
// eslint-disable-next-line import/no-absolute-path
Copy link
Member

Choose a reason for hiding this comment

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

If we allow this, why not add it as a global rule?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's some junk for a test that we otherwise wouldn't want i real tracer code

const app = require('/opt/insecure-bank-js/app')

const { port } = require('./common')
Expand Down
1 change: 1 addition & 0 deletions benchmark/sirun/appsec/insecure-bank.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const http = require('http')
// eslint-disable-next-line import/no-absolute-path
const app = require('/opt/insecure-bank-js/app')

const { port } = require('./common')
Expand Down
1 change: 0 additions & 1 deletion benchmark/sirun/scope/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const {
DD_TRACE_SCOPE,
COUNT
Expand Down
3 changes: 3 additions & 0 deletions integration-tests/ci-visibility/features/support/steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ class Greeter {
sayFarewell () {
return 'farewell'
}

sayGreetings () {
return 'greetings'
}

sayYo () {
return 'yo'
}

sayYeah () {
return 'yeah whatever'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ require('dd-trace').init({
service: 'dd-trace-bad-init'
})

// eslint-disable-next-line import/no-extraneous-dependencies
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, if this is allowed in the project, why not add it as a global rule?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's some junk for a test that we otherwise wouldn't want i real tracer code

Copy link
Member

Choose a reason for hiding this comment

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

Could always add a .eslintrc override at the root folder of the tests.

module.exports = require('jest-environment-node')
1 change: 1 addition & 0 deletions integration-tests/ci-visibility/run-jest-bad-init.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// eslint-disable-next-line import/no-extraneous-dependencies
const jest = require('jest')

const options = {
Expand Down
1 change: 1 addition & 0 deletions integration-tests/ci-visibility/run-jest.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// eslint-disable-next-line import/no-extraneous-dependencies
const jest = require('jest')

const options = {
Expand Down
2 changes: 2 additions & 0 deletions node-upstream-tests/node/run_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ class TestResult {
this.isPass = null
this.isIgnore = null
}

async init () {
this.isPass = this.rc === 0

Expand All @@ -257,6 +258,7 @@ class TestResult {

return this
}

errorMessage () {
let message = ''
message += `Test output: rc ${this.rc}\n`
Expand Down
7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,11 @@
"cli-table3": "^0.6.3",
"dotenv": "16.3.1",
"esbuild": "0.16.12",
"eslint": "^8.23.0",
"eslint-config-standard": "^11.0.0-beta.0",
"eslint": "^8.56.0",
"eslint-config-standard": "^17",
"eslint-plugin-import": "^2.8.0",
"eslint-plugin-mocha": "^10.1.0",
"eslint-plugin-n": "^15.7.0",
"eslint-plugin-node": "^5.2.1",
"eslint-plugin-n": "^16.6.2",
Copy link
Member

Choose a reason for hiding this comment

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

This version doesn't support older versions of Node. Security fixes should be backportable to all release lines.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... Do we run the linter with every supported version of Node?

Copy link
Member

Choose a reason for hiding this comment

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

In some cases yes.

"eslint-plugin-promise": "^3.6.0",
"eslint-plugin-standard": "^3.0.1",
"express": "^4.18.2",
Expand Down
1 change: 0 additions & 1 deletion packages/datadog-instrumentations/src/fs.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

'use strict'

const {
Expand Down
1 change: 1 addition & 0 deletions packages/datadog-plugin-couchbase/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class CouchBasePlugin extends StoragePlugin {
this._addCommandSubs('append')
this._addCommandSubs('prepend')
}

_addCommandSubs (name) {
this.addSubs(name, ({ bucket, collection, seedNodes }) => {
const store = storage.getStore()
Expand Down
4 changes: 2 additions & 2 deletions packages/datadog-plugin-fs/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1348,11 +1348,11 @@ describe('Plugin', () => {
'file.path': __filename
}
})
fs.exists(__filename, () => {}) // eslint-disable-line node/no-deprecated-api
fs.exists(__filename, () => {}) // eslint-disable-line n/no-deprecated-api
})

it('should support promisification', () => {
const exists = util.promisify(fs.exists) // eslint-disable-line node/no-deprecated-api
const exists = util.promisify(fs.exists) // eslint-disable-line n/no-deprecated-api

return exists(__filename)
})
Expand Down
1 change: 1 addition & 0 deletions packages/datadog-plugin-http2/test/server.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class MockAbortController {
constructor () {
this.signal = new EventEmitter()
}

abort () {
this.signal.emit('abort')
}
Expand Down
1 change: 1 addition & 0 deletions packages/datadog-plugin-jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class JestPlugin extends CiPlugin {
}
return this.unskippableSuites
}

getForcedToRunSuites (forcedToRunSuitesList) {
if (!this.forcedToRunSuites) {
this.forcedToRunSuites = JSON.parse(forcedToRunSuitesList)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
checkSpansForServiceName,
spawnPluginIntegrationTestProc
} = require('../../../../integration-tests/helpers')
// eslint-disable-next-line import/no-extraneous-dependencies
const { assert } = require('chai')
const { NODE_MAJOR } = require('../../../../version')

Expand Down
1 change: 1 addition & 0 deletions packages/datadog-plugin-next/test/next.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Build config dynamically for ease in testing and modification

// eslint-disable-next-line import/no-extraneous-dependencies
const { satisfies } = require('semver')

const { VERSION } = process.env // Next.js version to dynamically modify parts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class CookieAnalyzer extends Analyzer {
_getExcludedPaths () {
return EXCLUDED_PATHS
}

_checkOCE (context, value) {
if (value && value.location) {
return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class HstsHeaderMissingAnalyzer extends MissingHeaderAnalyzer {
constructor () {
super(HSTS_HEADER_MISSING, HSTS_HEADER_NAME)
}

_isVulnerableFromRequestAndResponse (req, res) {
const headerValues = this._getHeaderValues(res, HSTS_HEADER_NAME)
return this._isHttpsProtocol(req) && (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const fs = require('fs')
const path = require('path')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class TestApiManualPlugin extends CiPlugin {
static get id () {
return 'test-api-manual'
}

constructor (...args) {
super(...args)
this.sourceRoot = process.cwd()
Expand Down
1 change: 0 additions & 1 deletion packages/dd-trace/src/exporters/span-stats/writer.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const { SpanStatsEncoder } = require('../../encode/span-stats')

const pkg = require('../../../../../package.json')
Expand Down
2 changes: 2 additions & 0 deletions packages/dd-trace/src/opentelemetry/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@ class Span {
get resource () {
return this._parentTracer.resource
}

get instrumentationLibrary () {
return this._parentTracer.instrumentationLibrary
}

get _spanProcessor () {
return this._parentTracer.getActiveSpanProcessor()
}
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/plugins/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class DatabasePlugin extends StoragePlugin {
encodedDdpv: ''
}
}

encodingServiceTags (serviceTag, encodeATag, spanConfig) {
if (serviceTag !== spanConfig) {
this.serviceTags[serviceTag] = spanConfig
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

function configWithFallback ({ tracerService, pluginConfig }) {
return pluginConfig.service || tracerService
}
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/span_processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class SpanProcessor {
}

if (this._killAll) {
// eslint-disable-next-line array-callback-return
Copy link
Member

Choose a reason for hiding this comment

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

Why is it a .map then? Similar question for all other occurrences.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the author intended to use a forEach?

Copy link
Member

Choose a reason for hiding this comment

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

Then I'd say to change the code instead of adding an eslint rule.

started.map(startedSpan => {
if (!startedSpan._finished) {
startedSpan.finish()
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/telemetry/dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ let updateRetryData

function createBatchPayload (payload) {
const batchPayload = []
// eslint-disable-next-line array-callback-return
payload.map(item => {
batchPayload.push({
request_type: item.reqType,
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/telemetry/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ function getTelemetryData () {

function createBatchPayload (payload) {
const batchPayload = []
// eslint-disable-next-line array-callback-return
payload.map(item => {
batchPayload.push({
request_type: item.reqType,
Expand Down
1 change: 0 additions & 1 deletion packages/dd-trace/src/telemetry/send-data.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const request = require('../exporters/common/request')
const log = require('../log')
const { isTrue } = require('../util')
Expand Down
14 changes: 7 additions & 7 deletions packages/dd-trace/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ function globMatch (pattern, subject) {
if (px < pattern.length) {
const c = pattern[px]
switch (c) {
default: // ordinary character
if (sx < subject.length && subject[sx] === c) {
px++
sx++
continue
}
break
case '?':
if (sx < subject.length) {
px++
Expand All @@ -51,6 +44,13 @@ function globMatch (pattern, subject) {
nextSx = sx + 1
px++
continue
default: // ordinary character
if (sx < subject.length && subject[sx] === c) {
px++
sx++
continue
}
break
}
}
if (nextSx > 0 && nextSx <= subject.length) {
Expand Down
2 changes: 2 additions & 0 deletions packages/dd-trace/test/appsec/iast/iast-plugin.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('IAST Plugin', () => {
addSub (channelName, handler) {
addSubMock(channelName, handler)
}

configure (config) {
configureMock(config)
}
Expand Down Expand Up @@ -191,6 +192,7 @@ describe('IAST Plugin', () => {
addSub (channelName, handler) {
addSubMock(channelName, handler)
}

configure (config) {
configureMock(config)
}
Expand Down
3 changes: 3 additions & 0 deletions packages/dd-trace/test/appsec/iast/path-line.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ class CallSiteMock {
this.lineNumber = lineNumber
this.columnNumber = columnNumber
}

getLineNumber () {
return this.lineNumber
}

getColumnNumber () {
return this.columnNumber
}

getFileName () {
return this.fileName
}
Expand Down
2 changes: 2 additions & 0 deletions packages/dd-trace/test/proxy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,11 @@ describe('TracerProxy', () => {
constructor (cfg) {
dogstatsdConfig = cfg
}

increment () {
dogstatsdIncrements.push(arguments)
}

flush () {
dogstatsdFlushes++
}
Expand Down
1 change: 1 addition & 0 deletions scripts/check-proposal-labels.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ ${withoutExclusionStderr}
const commitsHashesWithoutExclusions = withoutExclusionStdout.split('\n')
if (commitsHashesWithExclusions.length !== commitsHashesWithoutExclusions.length) {
const commitsWithInvalidLabels = []
// eslint-disable-next-line array-callback-return
commitsHashesWithoutExclusions.filter(c1 => {
if (!commitsHashesWithExclusions.some(c2 => c2 === c1)) {
commitsWithInvalidLabels.push(c1)
Expand Down
Loading
Loading