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

ESLINTJS-53 Fix incompatibilities with ESLint 9 #4818

Merged
merged 12 commits into from
Sep 19, 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
13 changes: 13 additions & 0 deletions its/eslint9-plugin-sonarjs/file.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,16 @@ function S125() {
*/

// if (something) {}
import { useEffect, useState } from 'react';

function Form() {
const [name, setName] = useState('Mary');
if (name !== '') {
useEffect(function persistForm() {
// Noncompliant {{React Hook "useEffect" is called conditionally. React Hooks must be called in the exact same order in every component render.}}
// ^^^^^^^^^
console.log('persistForm');
});
}
return 1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void test() throws Exception {
extractArchive(fileToExtract, temp);
bridge.start(temp);
assertStatus(bridge);
bridge.request(gson.toJson(InitLinter.build("sonar-no-unused-vars")), "init-linter");
bridge.request(gson.toJson(InitLinter.build("S1481")), "init-linter");
assertAnalyzeJs(bridge);
} finally {
bridge.stop();
Expand Down
24 changes: 12 additions & 12 deletions packages/bridge/tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('router', () => {
const payload: ProjectAnalysisInput = {
rules: [
{
key: 'no-duplicate-in-composite',
key: 'S4621',
configurations: [],
fileTypeTarget: ['MAIN'],
language: 'ts',
Expand All @@ -85,7 +85,7 @@ describe('router', () => {
} = JSON.parse(response);
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'no-duplicate-in-composite',
ruleId: 'S4621',
line: 1,
column: 28,
endLine: 1,
Expand Down Expand Up @@ -114,7 +114,7 @@ describe('router', () => {

it('should route /analyze-js requests', async () => {
await requestInitLinter(server, [
{ key: 'prefer-regex-literals', configurations: [], fileTypeTarget: ['MAIN'] },
{ key: 'S6325', configurations: [], fileTypeTarget: ['MAIN'] },
]);
const filePath = path.join(fixtures, 'file.js');
const fileType = 'MAIN';
Expand All @@ -126,7 +126,7 @@ describe('router', () => {
} = responseData;
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'prefer-regex-literals',
ruleId: 'S6325',
line: 1,
column: 0,
endLine: 1,
Expand All @@ -144,7 +144,7 @@ describe('router', () => {

it('should route /analyze-ts requests', async () => {
await requestInitLinter(server, [
{ key: 'no-duplicate-in-composite', configurations: [], fileTypeTarget: ['MAIN'] },
{ key: 'S4621', configurations: [], fileTypeTarget: ['MAIN'] },
]);
const filePath = path.join(fixtures, 'file.ts');
const fileType = 'MAIN';
Expand All @@ -156,7 +156,7 @@ describe('router', () => {
} = JSON.parse(response);
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'no-duplicate-in-composite',
ruleId: 'S4621',
line: 1,
column: 28,
endLine: 1,
Expand All @@ -168,7 +168,7 @@ describe('router', () => {

it('should route /analyze-with-program requests', async () => {
await requestInitLinter(server, [
{ key: 'no-duplicate-in-composite', configurations: [], fileTypeTarget: ['MAIN'] },
{ key: 'S4621', configurations: [], fileTypeTarget: ['MAIN'] },
]);
const filePath = path.join(fixtures, 'file.ts');
const fileType = 'MAIN';
Expand All @@ -183,7 +183,7 @@ describe('router', () => {
} = JSON.parse(response);
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'no-duplicate-in-composite',
ruleId: 'S4621',
line: 1,
column: 28,
endLine: 1,
Expand All @@ -195,7 +195,7 @@ describe('router', () => {

it('should route /analyze-yaml requests', async () => {
await requestInitLinter(server, [
{ key: 'no-all-duplicated-branches', configurations: [], fileTypeTarget: ['MAIN'] },
{ key: 'S3923', configurations: [], fileTypeTarget: ['MAIN'] },
]);
const filePath = path.join(fixtures, 'file.yaml');
const data = { filePath };
Expand All @@ -204,7 +204,7 @@ describe('router', () => {
issues: [issue],
} = JSON.parse(response);
expect(issue).toEqual({
ruleId: 'no-all-duplicated-branches',
ruleId: 'S3923',
line: 8,
column: 17,
endLine: 8,
Expand All @@ -218,7 +218,7 @@ describe('router', () => {

it('should route /analyze-html requests', async () => {
await requestInitLinter(server, [
{ key: 'no-all-duplicated-branches', configurations: [], fileTypeTarget: ['MAIN'] },
{ key: 'S3923', configurations: [], fileTypeTarget: ['MAIN'] },
]);
const filePath = path.join(fixtures, 'file.html');
const data = { filePath };
Expand All @@ -227,7 +227,7 @@ describe('router', () => {
issues: [issue],
} = JSON.parse(response);
expect(issue).toEqual({
ruleId: 'no-all-duplicated-branches',
ruleId: 'S3923',
line: 10,
column: 2,
endLine: 10,
Expand Down
4 changes: 2 additions & 2 deletions packages/bridge/tests/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('server', () => {

const { server, serverClosed } = await start(port);

const ruleId = 'no-extra-semi';
const ruleId = 'S1116';
const fileType = 'MAIN';

expect(JSON.parse(await requestAnalyzeJs(server, fileType))).toStrictEqual({
Expand Down Expand Up @@ -95,7 +95,7 @@ describe('server', () => {

expect(server.listening).toBeTruthy();

const ruleId = 'no-extra-semi';
const ruleId = 'S1116';
const fileType = 'MAIN';

await requestInitLinter(server, fileType, ruleId);
Expand Down
20 changes: 7 additions & 13 deletions packages/html/tests/analysis/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ describe('analyzeHTML', () => {
});

it('should analyze HTML file', async () => {
initializeLinter([
{ key: 'no-all-duplicated-branches', configurations: [], fileTypeTarget: ['MAIN'] },
]);
initializeLinter([{ key: 'S3923', configurations: [], fileTypeTarget: ['MAIN'] }]);
const {
issues: [issue],
} = analyzeEmbedded(
Expand All @@ -49,7 +47,7 @@ describe('analyzeHTML', () => {
);
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'no-all-duplicated-branches',
ruleId: 'S3923',
line: 10,
column: 2,
endLine: 10,
Expand All @@ -59,7 +57,7 @@ describe('analyzeHTML', () => {
});

it('should not break when using a rule with a quickfix', async () => {
initializeLinter([{ key: 'no-extra-semi', configurations: [], fileTypeTarget: ['MAIN'] }]);
initializeLinter([{ key: 'S1116', configurations: [], fileTypeTarget: ['MAIN'] }]);
const result = analyzeEmbedded(
await embeddedInput({ filePath: join(fixturesPath, 'quickfix.html') }),
parseHTML,
Expand All @@ -85,10 +83,10 @@ describe('analyzeHTML', () => {
]);
});

it('should not break when using "enforce-trailing-comma" rule', async () => {
it('should not break when using "S3723" rule', async () => {
initializeLinter([
{
key: 'enforce-trailing-comma',
key: 'S3723',
configurations: ['always-multiline'],
fileTypeTarget: ['MAIN'],
},
Expand Down Expand Up @@ -117,9 +115,7 @@ describe('analyzeHTML', () => {
});

it('should not break when using a rule with secondary locations', async () => {
initializeLinter([
{ key: 'for-loop-increment-sign', configurations: [], fileTypeTarget: ['MAIN'] },
]);
initializeLinter([{ key: 'S2251', configurations: [], fileTypeTarget: ['MAIN'] }]);
const result = analyzeEmbedded(
await embeddedInput({ filePath: join(fixturesPath, 'secondary.html') }),
parseHTML,
Expand All @@ -140,9 +136,7 @@ describe('analyzeHTML', () => {
});

it('should not break when using a regex rule', async () => {
initializeLinter([
{ key: 'sonar-no-regex-spaces', configurations: [], fileTypeTarget: ['MAIN'] },
]);
initializeLinter([{ key: 'S6326', configurations: [], fileTypeTarget: ['MAIN'] }]);
const result = analyzeEmbedded(
await embeddedInput({ filePath: join(fixturesPath, 'regex.html') }),
parseHTML,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ import { analyzeWithWatchProgram } from './analyzeWithWatchProgram';
import { analyzeWithoutProgram } from './analyzeWithoutProgram';
import { initializeLinter } from '../../linter';
import { TSCONFIG_JSON, setTSConfigs, getTSConfigsIterator } from '../../program';
import { PACKAGE_JSON, parsePackageJson, setPackageJsons, File, searchFiles } from '../../rules';
import {
PACKAGE_JSON,
parsePackageJson,
setPackageJsons,
File,
searchFiles,
} from '../../rules/helpers';
import { toUnixPath } from '@sonar/shared';

/**
Expand Down
37 changes: 2 additions & 35 deletions packages/jsts/src/linter/bundle-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
import { Linter, Rule } from 'eslint';
import { eslintRules } from '../rules/core';
import { tsEslintRules } from '../rules/typescript-eslint';
import { rules as reactESLintRules } from 'eslint-plugin-react';
import { rules as reactA11yRules } from 'eslint-plugin-jsx-a11y';
import { rules as importRules } from 'eslint-plugin-import';
import { rules as internalRules } from '../rules';
import { rules as internalRules } from '../rules/index';
import { customRules as internalCustomRules, CustomRule } from './custom-rules';
import { debug, getContext } from '@sonar/shared';

Expand All @@ -44,42 +39,14 @@ export function loadBundles(linter: Linter, rulesBundles: (keyof typeof loaders)
* different data structure (array/record/object).
*/
const loaders: { [key: string]: Function } = {
/**
* Loads external rules
*
* The external ESLint-based rules include all the rules that are
* not implemented internally, in other words, rules from external
* dependencies which include ESLint core rules.
*/
externalRules(linter: Linter) {
const externalRules: { [key: string]: Rule.RuleModule } = {};
/**
* The order of defining rules from external dependencies is important here.
* Core ESLint rules could be overridden by the implementation from specific
* dependencies, which should be the default behaviour in most cases.
*/
const dependencies = [
eslintRules,
tsEslintRules,
reactESLintRules,
reactA11yRules,
importRules,
];
for (const dependencyRules of dependencies) {
for (const [name, module] of Object.entries(dependencyRules)) {
externalRules[name] = module;
}
}
linter.defineRules(externalRules);
},
/**
* Loads internal rules
*
* Adds the rules from SonarJS plugin, i.e. rules in path
* /src/rules
*/
internalRules(linter: Linter) {
linter.defineRules(internalRules);
linter.defineRules(internalRules as unknown as { [name: string]: Rule.RuleModule });
},
/**
* Loads global context rules
Expand Down
2 changes: 1 addition & 1 deletion packages/jsts/src/linter/issues/decode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { Rule } from 'eslint';
import { Issue } from './issue';
import { hasSonarRuntimeOption } from '../parameters';
import { type EncodedMessage } from '../../rules';
import { type EncodedMessage } from '../../rules/helpers';

/**
* Decodes an issue with secondary locations, if any
Expand Down
34 changes: 0 additions & 34 deletions packages/jsts/src/linter/quickfixes/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,40 +25,6 @@
* rule that includes a fix.
*/
const quickFixMessages = new Map<string, string>([
['comma-dangle', 'Remove this trailing comma'],
['eol-last', 'Add a new line at the end of file'],
['jsx-no-useless-fragment', 'Remove redundant fragment'],
['no-absolute-path', 'Replace with a relative path'],
['no-empty-interface', 'Replace with type alias'],
['no-extra-bind', 'Remove .bind() call'],
['no-extra-boolean-cast', 'Remove extra cast'],
['no-extra-semi', 'Remove extra semicolon'],
['no-inferrable-types', 'Remove type declaration'],
['no-lonely-if', "Replace with 'else if'"],
['no-non-null-assertion', "Replace with optional chaining '.?'"],
['no-trailing-spaces', 'Remove trailing space'],
['no-undef-init', 'Remove initialization'],
['no-unnecessary-type-arguments', 'Remove type argument'],
['no-unnecessary-type-assertion', 'Remove type assertion'],
['no-unneeded-ternary', 'Replace with a simpler expression'],
['no-useless-rename', 'Remove alias'],
['no-var', "Replace 'var' with 'let'"],
['object-shorthand', 'Use shorthand property'],
['prefer-as-const', "Replace with 'as const'"],
['prefer-const', "Replace with 'const'"],
['prefer-function-type', 'Replace with a function type'],
['prefer-immediate-return', 'Return value immediately'],
['prefer-namespace-keyword', "Replace with 'namespace' keyword"],
['prefer-object-has-own', 'Replace with Object.hasOwn()'],
['prefer-object-spread', 'Replace with object spread syntax'],
['prefer-readonly', "Add 'readonly'"],
['prefer-return-this-type', "Replace return type with 'this'"],
['prefer-template', 'Replace with template string literal'],
['prefer-while', "Replace with 'while' loop"],
['quotes', 'Fix quotes'],
['radix', 'Add 10 as radix'],
['semi', 'Add semicolon'],

['S1537', 'Remove this trailing comma'],
['S113', 'Add a new line at the end of file'],
['S6749', 'Remove redundant fragment'],
Expand Down
Loading