Skip to content

Commit

Permalink
core: add original location to most usages of source-location (#13393)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark authored Nov 24, 2021
1 parent 62a552f commit 5618037
Show file tree
Hide file tree
Showing 22 changed files with 311 additions and 171 deletions.
46 changes: 39 additions & 7 deletions lighthouse-core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,21 +233,53 @@ class Audit {
}

/**
* @param {LH.Artifacts.ConsoleMessage} entry
* @return {LH.Audit.Details.SourceLocationValue | undefined}
* @param {LH.Artifacts.Bundle} bundle
* @param {number} generatedLine
* @param {number} generatedColumn
* @return {LH.Audit.Details.SourceLocationValue['original']}
*/
static makeSourceLocationFromConsoleMessage(entry) {
if (!entry.url) return;
static _findOriginalLocation(bundle, generatedLine, generatedColumn) {
const entry = bundle?.map.findEntry(generatedLine, generatedColumn);
if (!entry) return;

return {
file: entry.sourceURL || '',
line: entry.sourceLineNumber || 0,
column: entry.sourceColumnNumber || 0,
};
}

/**
* @param {string} url
* @param {number} line 0-indexed
* @param {number} column 0-indexed
* @param {LH.Artifacts.Bundle=} bundle
* @return {LH.Audit.Details.SourceLocationValue}
*/
static makeSourceLocation(url, line, column, bundle) {
return {
type: 'source-location',
url: entry.url,
url,
urlProvider: 'network',
line: entry.lineNumber || 0,
column: entry.columnNumber || 0,
line,
column,
original: bundle && this._findOriginalLocation(bundle, line, column),
};
}

/**
* @param {LH.Artifacts.ConsoleMessage} entry
* @param {LH.Artifacts.Bundle=} bundle
* @return {LH.Audit.Details.SourceLocationValue | undefined}
*/
static makeSourceLocationFromConsoleMessage(entry, bundle) {
if (!entry.url) return;

const line = entry.lineNumber || 0;
const column = entry.columnNumber || 0;
return this.makeSourceLocation(entry.url, line, column, bundle);
}

/**
* @param {number|null} score
* @param {LH.Audit.ScoreDisplayMode} scoreDisplayMode
Expand Down
27 changes: 1 addition & 26 deletions lighthouse-core/audits/byte-efficiency/legacy-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -397,23 +397,6 @@ class LegacyJavascript extends ByteEfficiencyAudit {
return transferRatio;
}

/**
* @param {LH.Artifacts.Bundle} bundle
* @param {number} generatedLine
* @param {number} generatedColumn
* @return {LH.Audit.Details.SourceLocationValue['original']}
*/
static _findOriginalLocation(bundle, generatedLine, generatedColumn) {
const entry = bundle && bundle.map.findEntry(generatedLine, generatedColumn);
if (!entry) return;

return {
file: entry.sourceURL || '',
line: entry.sourceLineNumber || 0,
column: entry.sourceColumnNumber || 0,
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
Expand Down Expand Up @@ -456,18 +439,10 @@ class LegacyJavascript extends ByteEfficiencyAudit {
const bundle = bundles.find(bundle => bundle.script.src === url);
for (const match of matches) {
const {name, line, column} = match;

/** @type {SubItem} */
const subItem = {
signal: name,
location: {
type: 'source-location',
url,
line,
column,
original: bundle && this._findOriginalLocation(bundle, line, column),
urlProvider: 'network',
},
location: ByteEfficiencyAudit.makeSourceLocation(url, line, column, bundle),
};
item.subItems.items.push(subItem);
}
Expand Down
24 changes: 10 additions & 14 deletions lighthouse-core/audits/deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// TODO: when M97 is sufficiently old, drop support for console messages

const Audit = require('./audit.js');
const JsBundles = require('../computed/js-bundles.js');
const i18n = require('../lib/i18n/i18n.js');

const UIStrings = {
Expand Down Expand Up @@ -47,34 +48,29 @@ class Deprecations extends Audit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['ConsoleMessages', 'InspectorIssues'],
requiredArtifacts: ['ConsoleMessages', 'InspectorIssues', 'SourceMaps', 'ScriptElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
static async audit(artifacts, context) {
const entries = artifacts.ConsoleMessages;
const bundles = await JsBundles.request(artifacts, context);

let deprecations;

if (artifacts.InspectorIssues.deprecations.length) {
deprecations = artifacts.InspectorIssues.deprecations
.map(deprecation => {
const {url, lineNumber, columnNumber} = deprecation.sourceCodeLocation;
const bundle = bundles.find(bundle => bundle.script.src === url);
return {
value: deprecation.message || '',
/** @type {LH.Audit.Details.SourceLocationValue} */
source: {
type: 'source-location',
url: deprecation.sourceCodeLocation.url,
urlProvider: 'network',
line: deprecation.sourceCodeLocation.lineNumber,
// Protocol.Audits.SourceCodeLocation.columnNumber is 1-indexed,
// but we use 0-indexed.
column: deprecation.sourceCodeLocation.columnNumber - 1,
},
// Protocol.Audits.SourceCodeLocation.columnNumber is 1-indexed, but we use 0-indexed.
source: Audit.makeSourceLocation(url, lineNumber, columnNumber - 1, bundle),
};
});
} else {
Expand Down
9 changes: 5 additions & 4 deletions lighthouse-core/audits/dobetterweb/geolocation-on-start.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,18 @@ class GeolocationOnStart extends ViolationAudit {
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
supportedModes: ['navigation'],
requiredArtifacts: ['ConsoleMessages'],
requiredArtifacts: ['ConsoleMessages', 'SourceMaps', 'ScriptElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
static async audit(artifacts, context) {
// 'Only request geolocation information in response to a user gesture.'
const results = ViolationAudit.getViolationResults(artifacts, /geolocation/);
const results = await ViolationAudit.getViolationResults(artifacts, context, /geolocation/);

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
Expand Down
10 changes: 6 additions & 4 deletions lighthouse-core/audits/dobetterweb/no-document-write.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,18 @@ class NoDocWriteAudit extends ViolationAudit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['ConsoleMessages'],
requiredArtifacts: ['ConsoleMessages', 'SourceMaps', 'ScriptElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
const results = ViolationAudit.getViolationResults(artifacts, /document\.write/);
static async audit(artifacts, context) {
const results =
await ViolationAudit.getViolationResults(artifacts, context, /document\.write/);

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
Expand Down
10 changes: 6 additions & 4 deletions lighthouse-core/audits/dobetterweb/notification-on-start.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,18 @@ class NotificationOnStart extends ViolationAudit {
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
supportedModes: ['navigation'],
requiredArtifacts: ['ConsoleMessages'],
requiredArtifacts: ['ConsoleMessages', 'SourceMaps', 'ScriptElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
const results = ViolationAudit.getViolationResults(artifacts, /notification permission/);
static async audit(artifacts, context) {
const results =
await ViolationAudit.getViolationResults(artifacts, context, /notification permission/);

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,18 @@ class PassiveEventsAudit extends ViolationAudit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['ConsoleMessages'],
requiredArtifacts: ['ConsoleMessages', 'SourceMaps', 'ScriptElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
const results = ViolationAudit.getViolationResults(artifacts, /passive event listener/);
static async audit(artifacts, context) {
const results =
await ViolationAudit.getViolationResults(artifacts, context, /passive event listener/);

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
Expand Down
11 changes: 7 additions & 4 deletions lighthouse-core/audits/errors-in-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

const log = require('lighthouse-logger');
const Audit = require('./audit.js');
const JsBundles = require('../computed/js-bundles.js');
const i18n = require('../lib/i18n/i18n.js');

const UIStrings = {
Expand Down Expand Up @@ -39,7 +40,7 @@ class ErrorLogs extends Audit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['ConsoleMessages'],
requiredArtifacts: ['ConsoleMessages', 'SourceMaps', 'ScriptElements'],
};
}

Expand Down Expand Up @@ -75,20 +76,22 @@ class ErrorLogs extends Audit {
/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {LH.Audit.Product}
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts, context) {
static async audit(artifacts, context) {
/** @type {AuditOptions} */
const auditOptions = context.options;
const bundles = await JsBundles.request(artifacts, context);

/** @type {Array<{source: string, description: string|undefined, sourceLocation: LH.Audit.Details.SourceLocationValue|undefined}>} */
const consoleRows = artifacts.ConsoleMessages
.filter(item => item.level === 'error')
.map(item => {
const bundle = bundles.find(bundle => bundle.script.src === item.url);
return {
source: item.source,
description: item.text,
sourceLocation: Audit.makeSourceLocationFromConsoleMessage(item),
sourceLocation: Audit.makeSourceLocationFromConsoleMessage(item, bundle),
};
});

Expand Down
19 changes: 9 additions & 10 deletions lighthouse-core/audits/no-unload-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'use strict';

const Audit = require('./audit.js');
const JsBundles = require('../computed/js-bundles.js');
const i18n = require('./../lib/i18n/i18n.js');

const UIStrings = {
Expand All @@ -29,22 +30,25 @@ class NoUnloadListeners extends Audit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['GlobalListeners', 'JsUsage'],
requiredArtifacts: ['GlobalListeners', 'JsUsage', 'SourceMaps', 'ScriptElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
static async audit(artifacts, context) {
const unloadListeners = artifacts.GlobalListeners.filter(l => l.type === 'unload');
if (!unloadListeners.length) {
return {
score: 1,
};
}

const bundles = await JsBundles.request(artifacts, context);

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
Expand Down Expand Up @@ -74,14 +78,9 @@ class NoUnloadListeners extends Audit {
};
}

const bundle = bundles.find(bundle => bundle.script.src === url);
return {
source: {
type: 'source-location',
url,
urlProvider: 'network',
line: listener.lineNumber,
column: listener.columnNumber,
},
source: Audit.makeSourceLocation(url, listener.lineNumber, listener.columnNumber, bundle),
};
});

Expand Down
6 changes: 4 additions & 2 deletions lighthouse-core/audits/seo/font-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,11 @@ function findStyleRuleSource(baseURL, styleDeclaration, parentNode) {
}
}

const url = stylesheet.sourceURL;
const source = Audit.makeSourceLocation(stylesheet.sourceURL, line, column);
source.urlProvider = urlProvider;

return {
source: {type: 'source-location', url, urlProvider, line, column},
source,
selector,
};
}
Expand Down
13 changes: 10 additions & 3 deletions lighthouse-core/audits/violation-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@
'use strict';

const Audit = require('./audit.js');
const JsBundles = require('../computed/js-bundles.js');

class ViolationAudit extends Audit {
/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @param {RegExp} pattern
* @return {Array<{source: LH.Audit.Details.SourceLocationValue}>}
* @return {Promise<Array<{source: LH.Audit.Details.SourceLocationValue}>>}
*/
static getViolationResults(artifacts, pattern) {
static async getViolationResults(artifacts, context, pattern) {
const bundles = await JsBundles.request(artifacts, context);

/**
* @template T
* @param {T} value
Expand All @@ -26,7 +30,10 @@ class ViolationAudit extends Audit {
const seen = new Set();
return artifacts.ConsoleMessages
.filter(entry => entry.url && entry.source === 'violation' && pattern.test(entry.text))
.map(Audit.makeSourceLocationFromConsoleMessage)
.map(entry => {
const bundle = bundles.find(bundle => bundle.script.src === entry.url);
return Audit.makeSourceLocationFromConsoleMessage(entry, bundle);
})
.filter(filterUndefined)
.filter(source => {
// Filter out duplicate entries since they are not differentiable to the user
Expand Down
Loading

0 comments on commit 5618037

Please sign in to comment.