From 2b5753f245471900bcb39616cfbb831cee6bc97c Mon Sep 17 00:00:00 2001 From: Grace Date: Fri, 29 Jul 2022 10:10:59 -0700 Subject: [PATCH] @W-11446192@: checks if any files are processed by both eslint and eslint-lwc and emits warning --- messages/DefaultRuleManager.js | 3 ++- src/lib/DefaultRuleManager.ts | 28 ++++++++++++++++++++++++++- test/lib/RuleManager.test.ts | 35 +++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/messages/DefaultRuleManager.js b/messages/DefaultRuleManager.js index d1a440e3c..af170dd4e 100644 --- a/messages/DefaultRuleManager.js +++ b/messages/DefaultRuleManager.js @@ -1,6 +1,7 @@ module.exports = { "warning": { "targetSkipped": "Target: '%s' was not processed by any engines.", - "targetsSkipped": "Targets: '%s' were not processed by any engines." + "targetsSkipped": "Targets: '%s' were not processed by any engines.", + "pathsDoubleProcessed": "At least one file was processed by both ESLint and ESLint-LWC simultaneously, which could result in duplicate violations. Customize the targetPatterns property for eslint and eslint-lwc engines in %s to remove overlap on the following file(s): %s", } } diff --git a/src/lib/DefaultRuleManager.ts b/src/lib/DefaultRuleManager.ts index b9ee1c31e..e4c706ce7 100644 --- a/src/lib/DefaultRuleManager.ts +++ b/src/lib/DefaultRuleManager.ts @@ -14,7 +14,7 @@ import {Controller} from '../Controller'; import globby = require('globby'); import path = require('path'); import {uxEvents, EVENTS} from './ScannerEvents'; -import {CUSTOM_CONFIG} from '../Constants'; +import {CUSTOM_CONFIG, ENGINE, CONFIG_PILOT_FILE} from '../Constants'; Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'DefaultRuleManager'); @@ -80,6 +80,13 @@ export class DefaultRuleManager implements RuleManager { const runDescriptorList: RunDescriptor[] = []; const engines: RuleEngine[] = await this.resolveEngineFilters(filters, engineOptions); const matchedTargets: Set = new Set(); + + // Storing the paths from eslint and eslint-lwc to track if any are processed by both. + const paths: Map = new Map([ + [ENGINE.ESLINT, [] as string[]], + [ENGINE.ESLINT_LWC, [] as string[]], + ]); + for (const e of engines) { // For each engine, filter for the appropriate groups and rules and targets, and pass // them all in. Note that some engines (pmd) need groups while others (eslint) need the rules. @@ -107,8 +114,27 @@ export class DefaultRuleManager implements RuleManager { this.logger.trace(`${e.getName()} is not eligible to execute this time.`); } + if (paths.has(e.getName())) { + for (const t of engineTargets) { + for (const p of t.paths) { + paths.get(e.getName()).push(p); + } + } + } } + // Checking if any file paths were processed by eslint and eslint-lwc, which may cause duplicate violations. + const pathsDoubleProcessed = paths.get(ENGINE.ESLINT).filter(path => paths.get(ENGINE.ESLINT_LWC).includes(path)); + if (pathsDoubleProcessed.length > 0) { + const numFilesShown = 3; + const filesToDisplay = pathsDoubleProcessed.slice(0, numFilesShown); + if (pathsDoubleProcessed.length > numFilesShown) { + filesToDisplay.push(`and ${pathsDoubleProcessed.length - numFilesShown} more`) + } + uxEvents.emit(EVENTS.WARNING_ALWAYS, messages.getMessage('warning.pathsDoubleProcessed', [`${Controller.getSfdxScannerPath()}/${CONFIG_PILOT_FILE}`, `${filesToDisplay.join(', ')}`])); + } + + this.validateRunDescriptors(runDescriptorList); await this.emitRunTelemetry(runDescriptorList, runOptions.sfdxVersion); // Warn the user if any positive targets were skipped diff --git a/test/lib/RuleManager.test.ts b/test/lib/RuleManager.test.ts index c805cd6a9..3e736d5ee 100644 --- a/test/lib/RuleManager.test.ts +++ b/test/lib/RuleManager.test.ts @@ -4,7 +4,7 @@ import {Lifecycle} from '@salesforce/core'; import {Controller} from '../../src/Controller'; import {Rule, RuleGroup, RuleTarget, TelemetryData} from '../../src/types'; -import {ENGINE} from '../../src/Constants'; +import {ENGINE, CONFIG_PILOT_FILE} from '../../src/Constants'; import {CategoryFilter, EngineFilter, LanguageFilter, RuleFilter, RulesetFilter} from '../../src/lib/RuleFilter'; import {DefaultRuleManager} from '../../src/lib/DefaultRuleManager'; @@ -22,6 +22,10 @@ import * as TestUtils from '../TestUtils'; import path = require('path'); import Sinon = require('sinon'); +import {Messages} from '@salesforce/core'; +Messages.importMessagesDirectory(__dirname); +const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'DefaultRuleManager'); + TestOverrides.initializeTestSetup(); let ruleManager: RuleManager = null; @@ -272,6 +276,35 @@ describe('RuleManager', () => { Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, `Target: '${invalidTarget.join(', ')}' was not processed by any engines.`); Sinon.assert.callCount(telemetrySpy, 1); }); + + it('Warns correctly if eslint and eslint-lwc have one duplicate path to process', async () => { + const targets = ['../invalid-lwc']; + const filters: EngineFilter[] = [new EngineFilter(["eslint-lwc", "eslint", "pmd"])]; + + await ruleManager.runRulesMatchingCriteria(filters, targets, runOptions, EMPTY_ENGINE_OPTIONS); + + const filename = path.join(__dirname, '..','code-fixtures', 'invalid-lwc', 'invalidApiDecorator', 'noLeadingUpperCase.js') + const warningMessage = messages.getMessage('warning.pathsDoubleProcessed', [`${Controller.getSfdxScannerPath()}/${CONFIG_PILOT_FILE}`, `${filename}`]) + + Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, warningMessage); + Sinon.assert.callCount(telemetrySpy, 1); + }); + + it('Warns correctly if eslint and eslint-lwc have one duplicate path to process', async () => { + const targets = ["js", "../invalid-lwc"]; + const filters: EngineFilter[] = [new EngineFilter(["eslint-lwc", "eslint", "pmd"])]; + + await ruleManager.runRulesMatchingCriteria(filters, targets, runOptions, EMPTY_ENGINE_OPTIONS); + + const baseConfigEnv = path.join(__dirname, '..','code-fixtures', 'projects', 'js', 'src', 'baseConfigEnv.js') + const fileThatUsesQUnit = path.join(__dirname, '..','code-fixtures', 'projects', 'js', 'src', 'fileThatUsesQUnit.js') + const simpleYetWrong = path.join(__dirname, '..','code-fixtures', 'projects', 'js', 'src', 'simpleYetWrong.js') + const warningMessage = messages.getMessage('warning.pathsDoubleProcessed', [`${Controller.getSfdxScannerPath()}/${CONFIG_PILOT_FILE}`, `${baseConfigEnv}, ${fileThatUsesQUnit}, ${simpleYetWrong}, and 1 more`]) + + Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, warningMessage); + Sinon.assert.callCount(telemetrySpy, 1); + }); + }); describe('Test Case: Run by category', () => {