-
Notifications
You must be signed in to change notification settings - Fork 6
Bug fixes #60
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
Bug fixes #60
Changes from all commits
bb3540d
0930035
2dbc319
1b85ae9
7054421
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,59 +9,76 @@ | |
|
||
module.exports = { | ||
extends: ['./configs/base'], | ||
rules: { | ||
'@salesforce/lwc-graph-analyzer/no-getter-contains-more-than-return-statement': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-assignment-expression-assigns-value-to-member-variable': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-references-non-local-property-reactive-value': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-private-wire-config-property': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-unresolved-parent-class-reference': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-class-refers-to-parent-class-from-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-reference-to-unsupported-namespace-reference': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-uses-getter-function-returning-inaccessible-import': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-uses-getter-function-returning-non-literal': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-circular-wire-dependency': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-configuration-property-using-output-of-non-primeable-wire': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-missing-resource-cannot-prime-wire-adapter': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-uses-imported-artifact-from-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-adapter-of-resource-cannot-be-primed': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-unsupported-member-variable-in-member-expression': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-multiple-template-files': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-assignment-expression-for-external-components': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-tagged-template-expression-contains-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-expression-contains-module-level-variable-ref': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-call-expression-references-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-eval-usage': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-reference-to-class-functions': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-reference-to-module-functions': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-functions-declared-within-getter-method': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-non-existent-member-variable': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-unsupported-namespace-reference': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-contains-non-portable-identifier': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-super-class': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-unsupported-global': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-getter-property': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-property-from-unresolvable-wire': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-property-missing': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-property-non-public': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-render-function-contains-more-than-return-statement': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-render-function-return-statement-not-returning-imported-template': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-render-function-return-statement': 'warn' | ||
} | ||
overrides: [ | ||
{ | ||
files: ['*.html', '**/*.html', '*.js', '**/*.js'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I see the html extension here. |
||
processor: '@salesforce/lwc-graph-analyzer/bundleAnalyzer', | ||
rules: { | ||
'@salesforce/lwc-graph-analyzer/no-getter-contains-more-than-return-statement': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-assignment-expression-assigns-value-to-member-variable': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-references-non-local-property-reactive-value': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-private-wire-config-property': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-unresolved-parent-class-reference': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-class-refers-to-parent-class-from-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-reference-to-unsupported-namespace-reference': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-uses-getter-function-returning-inaccessible-import': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-uses-getter-function-returning-non-literal': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-circular-wire-dependency': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-configuration-property-using-output-of-non-primeable-wire': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-missing-resource-cannot-prime-wire-adapter': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-config-property-uses-imported-artifact-from-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-wire-adapter-of-resource-cannot-be-primed': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-unsupported-member-variable-in-member-expression': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-multiple-template-files': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-assignment-expression-for-external-components': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-tagged-template-expression-contains-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-expression-contains-module-level-variable-ref': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-call-expression-references-unsupported-namespace': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-eval-usage': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-reference-to-class-functions': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-reference-to-module-functions': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-functions-declared-within-getter-method': 'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-non-existent-member-variable': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-unsupported-namespace-reference': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-contains-non-portable-identifier': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-super-class': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-member-expression-reference-to-unsupported-global': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-getter-property': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-property-from-unresolvable-wire': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-property-missing': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-composition-on-unanalyzable-property-non-public': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-render-function-contains-more-than-return-statement': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-render-function-return-statement-not-returning-imported-template': | ||
'warn', | ||
'@salesforce/lwc-graph-analyzer/no-render-function-return-statement': 'warn' | ||
} | ||
} | ||
] | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,8 +8,13 @@ | |
'use strict'; | ||
|
||
const { extname } = require('path'); | ||
const { setLwcBundleCacheEntry, removeLwcBundleCacheEntry } = require('./util/helper'); | ||
const { | ||
setLwcBundleCacheEntry, | ||
removeLwcBundleCacheEntry, | ||
extractBundleKey | ||
} = require('./util/helper'); | ||
const LwcBundle = require('./lwc-bundle'); | ||
const bundleStateManager = require('./util/bundle-state-manager'); | ||
|
||
const SUPPORTS_AUTOFIX = true; | ||
|
||
|
@@ -83,6 +88,13 @@ class BundleAnalyzer { | |
* @returns {{ text: string, filename: string }[]} An array of files and their content, to be processed. | ||
*/ | ||
preprocess = (text, filename) => { | ||
// Check if this is a file we generated ourselves with `preprocess()` return values. | ||
const bundleKey = extractBundleKey(filename); | ||
if (bundleStateManager.hasBundleWithKey(bundleKey)) { | ||
// Skip processing for files we generated | ||
return [{ text, filename }]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this fix recursive processing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. The renaming we do at the bottom of the method causes ESLint to (re-)process this "new" file that we created. Conversely, when you return what you got in the processor, ESLint continues on with the rest of the process (i.e. considers that file pre-processed). |
||
} | ||
|
||
const fileExtension = extname(filename); | ||
|
||
// If this.#lwcBundle has already been set, use that data. Otherwise, create | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,14 +7,25 @@ | |
|
||
'use strict'; | ||
|
||
const { basename } = require('path'); | ||
const staticAnalyzer = require('@komaci/static-analyzer'); | ||
const { basename, extname } = require('path'); | ||
const bundleStateManager = require('./bundle-state-manager'); | ||
const StaticAnalyzerProvider = require('./static-analyzer-provider'); | ||
// eslint-disable-next-line no-unused-vars | ||
const LwcBundle = require('../lwc-bundle'); | ||
|
||
const lwcNamespace = 'c'; | ||
|
||
// Create a default provider instance | ||
let staticAnalyzerProvider = new StaticAnalyzerProvider(); | ||
|
||
/** | ||
* Sets the static analyzer provider to use | ||
* @param {import('./static-analyzer-interface')} provider - The provider to use | ||
*/ | ||
function setStaticAnalyzerProvider(provider) { | ||
staticAnalyzerProvider = provider; | ||
} | ||
|
||
function rangeToLoc(range) { | ||
const { | ||
start: { line, character: column }, | ||
|
@@ -63,7 +74,7 @@ function removeLwcBundleCacheEntry(bundle) { | |
} | ||
|
||
function analyzeLWC(context) { | ||
const eslintReports = getKomaciReport(context.id, context.filename); | ||
const eslintReports = getKomaciReport(context.id, context.filename, context.sourceCode.text); | ||
|
||
for (const report of eslintReports) { | ||
context.report(report); | ||
|
@@ -90,16 +101,28 @@ function extractBundleKey(eslintFilename) { | |
return filename.replace(/^(\d+_)?/, ''); | ||
} | ||
|
||
function getKomaciReport(ruleName, filename) { | ||
/** | ||
* Gets the Komaci diagnostic reports for a given rule and file. | ||
* Filters the reports to only include those that: | ||
* 1. Match the specified rule | ||
* 2. Target the primary file in the bundle | ||
* | ||
* @param {string} ruleName - The full ESLint rule name (e.g. '@salesforce/lwc-graph-analyzer/rule-name') | ||
* @param {string} filename - The filename being processed by ESLint | ||
* @param {string} sourceCode - The source code of the file being processed by ESLint | ||
* @returns {Array<Object>} An array of ESLint report objects, each containing a message and location information | ||
*/ | ||
function getKomaciReport(ruleName, filename, sourceCode) { | ||
const bundleKey = extractBundleKey(filename); | ||
const lwcBundle = bundleStateManager.getBundleByKey(bundleKey); | ||
let lwcBundle = bundleStateManager.getBundleByKey(bundleKey); | ||
if (!lwcBundle) { | ||
console.warn('getKomaciReport(): LWC bundle not configured. Nothing to do.'); | ||
return []; | ||
// We didn't stash a bundle for this file. Our processor may not have been invoked, due | ||
// to a clash with another processor. Just create a bundle for this file. | ||
lwcBundle = LwcBundle.lwcBundleFromFile(sourceCode, filename, extname(filename)); | ||
} | ||
|
||
const lwcBundleFiles = lwcBundle.filesRecord(); | ||
let eslintReports = staticAnalyzer.generatePrimingDiagnosticsModule({ | ||
let eslintReports = staticAnalyzerProvider.generatePrimingDiagnosticsModule({ | ||
type: 'bundle', | ||
namespace: lwcNamespace, | ||
name: lwcBundle.componentBaseName, | ||
|
@@ -115,7 +138,7 @@ function getKomaciReport(ruleName, filename) { | |
|
||
// Diagnostic messages is the catalog of all Komaci errors. Find the one that matches the rule name | ||
// so that the Komaci reports can be filtered. | ||
const diagnosticMessage = staticAnalyzer.diagnosticMessages[ruleKey]; | ||
const diagnosticMessage = staticAnalyzerProvider.diagnosticMessages[ruleKey]; | ||
|
||
if (!diagnosticMessage) { | ||
// No matching diagnostic message was found. Return an empty array to indicate that | ||
|
@@ -133,7 +156,11 @@ function getKomaciReport(ruleName, filename) { | |
eslintReports = eslintReports.filter((reportDiagnostic) => { | ||
const reportDiagnosticValue = extractDiagnosticCodeValue(reportDiagnostic); | ||
const diagnosticValue = extractDiagnosticCodeValue(diagnosticMessage); | ||
return reportDiagnosticValue === diagnosticValue; | ||
// Only include diagnostics that match both the rule and target the primary file | ||
return ( | ||
reportDiagnosticValue === diagnosticValue && | ||
reportDiagnostic.code.target.path === lwcBundle.primaryFile.filename | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started seeing a random squiggly line for an HTML violation, in the middle of my JS file. 😂 |
||
); | ||
}); | ||
|
||
return eslintReports.map(diagnosticToReport); | ||
|
@@ -142,5 +169,8 @@ function getKomaciReport(ruleName, filename) { | |
module.exports = { | ||
setLwcBundleCacheEntry, | ||
removeLwcBundleCacheEntry, | ||
analyzeLWC | ||
analyzeLWC, | ||
extractBundleKey, | ||
setStaticAnalyzerProvider, | ||
getKomaciReport | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* | ||
* Copyright (c) 2025, salesforce.com, inc. | ||
* All rights reserved. | ||
* SPDX-License-Identifier: MIT | ||
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT | ||
*/ | ||
|
||
'use strict'; | ||
|
||
/** | ||
* Interface for static analysis functionality | ||
*/ | ||
class StaticAnalyzerInterface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a mirror from Komaci library? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not per se. It was just a way to abstract the Komaci analysis functions we use, so that I could create a mock provider in test, to test how the analysis output impacted different parts of the reporting. You can see some of the mocked analysis tests below. |
||
constructor() { | ||
if (this.constructor === StaticAnalyzerInterface) { | ||
throw new TypeError( | ||
'StaticAnalyzerInterface is an abstract class and cannot be instantiated directly' | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* Generates priming diagnostics for a module | ||
* @param {Object} options - The options for generating diagnostics | ||
* @param {string} options.type - The type of module | ||
* @param {string} options.namespace - The namespace of the module | ||
* @param {string} options.name - The name of the module | ||
* @param {Object<string, string>} options.files - The files to analyze | ||
* @returns {Array<Object>} The generated diagnostics | ||
*/ | ||
// eslint-disable-next-line no-unused-vars | ||
generatePrimingDiagnosticsModule(options) { | ||
throw new Error('Method not implemented'); | ||
} | ||
|
||
/** | ||
* Gets the diagnostic messages catalog | ||
* @returns {Object} The diagnostic messages catalog | ||
*/ | ||
get diagnosticMessages() { | ||
throw new Error('Method not implemented'); | ||
} | ||
} | ||
|
||
module.exports = StaticAnalyzerInterface; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/* | ||
* Copyright (c) 2025, salesforce.com, inc. | ||
* All rights reserved. | ||
* SPDX-License-Identifier: MIT | ||
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT | ||
*/ | ||
|
||
'use strict'; | ||
|
||
const StaticAnalyzerInterface = require('./static-analyzer-interface'); | ||
const staticAnalyzer = require('@komaci/static-analyzer'); | ||
|
||
/** | ||
* Concrete implementation of StaticAnalyzerInterface that uses the actual static analyzer | ||
*/ | ||
class StaticAnalyzerProvider extends StaticAnalyzerInterface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pulled this out into its own interface implementation so we could test the outputs coming from the static analysis. |
||
generatePrimingDiagnosticsModule(options) { | ||
return staticAnalyzer.generatePrimingDiagnosticsModule(options); | ||
} | ||
|
||
get diagnosticMessages() { | ||
return staticAnalyzer.diagnosticMessages; | ||
} | ||
} | ||
|
||
module.exports = StaticAnalyzerProvider; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
html
overrides is removed, will any rule be applied to html files?