Skip to content

Commit

Permalink
tests(lint): require file extension in require() (#9017)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny authored May 23, 2019
1 parent 359b31f commit df85d68
Show file tree
Hide file tree
Showing 34 changed files with 164 additions and 79 deletions.
4 changes: 4 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = {
// start with google standard style
// https://github.com/google/eslint-config-google/blob/master/index.js
extends: ['eslint:recommended', 'google'],
plugins: ['eslint-plugin-local-rules'], // include custom rules
env: {
node: true,
es6: true,
Expand Down Expand Up @@ -64,6 +65,9 @@ module.exports = {
functions: 'never',
}],

// Custom lighthouse rules
'local-rules/require-file-extension': 2,

// Disabled rules
'require-jsdoc': 0,
'valid-jsdoc': 0,
Expand Down
6 changes: 3 additions & 3 deletions clients/extension/scripts/extension-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
*/
'use strict';

const lighthouse = require('../../../lighthouse-core/index');
const Config = require('../../../lighthouse-core/config/config');
const lighthouse = require('../../../lighthouse-core/index.js');
const Config = require('../../../lighthouse-core/config/config.js');
const defaultConfig = require('../../../lighthouse-core/config/default-config.js');
const i18n = require('../../../lighthouse-core/lib/i18n/i18n.js');

const ExtensionProtocol = require('../../../lighthouse-core/gather/connections/extension');
const ExtensionProtocol = require('../../../lighthouse-core/gather/connections/extension.js');
const log = require('lighthouse-logger');

/** @typedef {import('../../../lighthouse-core/gather/connections/connection.js')} Connection */
Expand Down
106 changes: 106 additions & 0 deletions eslint-local-rules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/**
* @license Copyright 2019 Google Inc. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/**
* @fileoverview A collection of eslint rules written specifically for
* Lighthouse. These are included by the eslint-plugin-local-rules plugin.
*/

const path = require('path');

/** @typedef {import('eslint').Rule.RuleModule} RuleModule */

/**
* Use `require.resolve()` to resolve the location of `path` from a location of
* `baseDir` and return it. Returns null if unable to resolve a path.
* @param {string} path
* @param {string} baseDir
* @return {string|null}
*/
function requireResolveOrNull(path, baseDir) {
try {
return require.resolve(path, {
paths: [baseDir],
});
} catch (err) {
return null;
}
}

/**
* An eslint rule ensuring that any require() of a local path (aka not a core
* module or a module dependency) includes a file extension (.js' or '.json').
* @type {RuleModule}
*/
const requireFileExtension = {
meta: {
docs: {
description: 'disallow require() without a file extension',
category: 'Best Practices',
recommended: false,
},
schema: [],
fixable: 'code',
},

create(context) {
return {
CallExpression(node) {
// Only look at instances of `require(moduleName: string)`.
if (node.type !== 'CallExpression') return;
if (node.callee.type !== 'Identifier' || node.callee.name !== 'require') return;
if (!node.arguments.length) return;
const arg0 = node.arguments[0];
if (arg0.type !== 'Literal' || typeof arg0.value !== 'string') return;

const requiredPath = arg0.value;

// If it's not a local file, we don't care.
if (!requiredPath.startsWith('.')) return;

// Check that `requiredPath` is resolvable from the source file.
const contextDirname = path.dirname(context.getFilename());
const resolvedRequiredPath = requireResolveOrNull(requiredPath, contextDirname);
if (!resolvedRequiredPath) {
return context.report({
node: node,
message: `Cannot resolve module '${requiredPath}'.`,
});
}

// If it has a file extension, it's good to go.
if (requiredPath.endsWith('.js')) return;
if (requiredPath.endsWith('.json')) return;

context.report({
node: node,
message: 'Local require path must have a file extension.',
fix(fixer) {
// Find the correct file extension/filename ending of the requiredPath.
let fixedPath = path.relative(contextDirname, resolvedRequiredPath);
if (!fixedPath.startsWith('.')) fixedPath = `./${fixedPath}`;

// Usually `fixedPath.startsWith(requiredPath)` and this will just add
// a suffix to the existing path, but sometimes humans write confusing
// paths, e.g. './lighthouse-core/lib/../lib/lh-error.js'. To cover both
// cases, double check that the paths resolve to the same file.
const resolvedFixedPath = requireResolveOrNull(fixedPath, contextDirname);

// If somehow they don't point to the same file, don't try to fix.
if (resolvedFixedPath !== resolvedRequiredPath) return null;

return fixer.replaceText(arg0, `'${fixedPath}'`);
},
});
},
};
},
};

module.exports = {
'require-file-extension': requireFileExtension,
};
2 changes: 1 addition & 1 deletion lighthouse-cli/commands/list-audits.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

const lighthouse = require('../../lighthouse-core/');
const lighthouse = require('../../lighthouse-core/index.js');

function listAudits() {
const audits = lighthouse.getAuditList().map((i) => i.replace(/\.js$/, ''));
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/commands/list-trace-categories.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

const lighthouse = require('../../lighthouse-core/');
const lighthouse = require('../../lighthouse-core/index.js');

function listTraceCategories() {
const traceCategories = lighthouse.traceCategories;
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/cli/cli-flags-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

/* eslint-env jest */
const assert = require('assert');
const getFlags = require('../../cli-flags').getFlags;
const getFlags = require('../../cli-flags.js').getFlags;

describe('CLI bin', function() {
it('all options should have descriptions', () => {
Expand Down
7 changes: 4 additions & 3 deletions lighthouse-cli/test/cli/run-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const path = require('path');
const fs = require('fs');

const run = require('../../run.js');
const parseChromeFlags = require('../../run').parseChromeFlags;
const parseChromeFlags = require('../../run.js').parseChromeFlags;
const fastConfig = {
'extends': 'lighthouse:default',
'settings': {
Expand All @@ -21,10 +21,11 @@ const fastConfig = {

// Map plugin name to fixture since not actually installed in node_modules/.
jest.mock('lighthouse-plugin-simple', () => {
return require('../../../lighthouse-core/test/fixtures/config-plugins/lighthouse-plugin-simple/');
// eslint-disable-next-line max-len
return require('../../../lighthouse-core/test/fixtures/config-plugins/lighthouse-plugin-simple/plugin-simple.js');
}, {virtual: true});

const getFlags = require('../../cli-flags').getFlags;
const getFlags = require('../../cli-flags.js').getFlags;

describe('CLI run', function() {
describe('LH round trip', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/pwa2-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

const pwaDetailsExpectations = require('./pwa-expectations').PWA_DETAILS_EXPECTATIONS;
const pwaDetailsExpectations = require('./pwa-expectations.js').PWA_DETAILS_EXPECTATIONS;

const jakeExpectations = {...pwaDetailsExpectations, hasShortName: false};

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/pwa3-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

const pwaDetailsExpectations = require('./pwa-expectations').PWA_DETAILS_EXPECTATIONS;
const pwaDetailsExpectations = require('./pwa-expectations.js').PWA_DETAILS_EXPECTATIONS;

const pwaRocksExpectations = {...pwaDetailsExpectations, hasIconsAtLeast512px: false};

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/run-smoke.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const log = require('lighthouse-logger');

/** @param {string} str */
const purpleify = str => `${log.purple}${str}${log.reset}`;
const SMOKETESTS = require('./smoke-test-dfns').SMOKE_TEST_DFNS;
const SMOKETESTS = require('./smoke-test-dfns.js').SMOKE_TEST_DFNS;

/**
* Display smokehouse output from child process
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const Audit = require('../audit.js');
const linearInterpolation = require('../../lib/statistics').linearInterpolation;
const linearInterpolation = require('../../lib/statistics.js').linearInterpolation;
const Interactive = require('../../computed/metrics/lantern-interactive.js');
const i18n = require('../../lib/i18n/i18n.js');
const NetworkRecords = require('../../computed/network-records.js');
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/byte-efficiency/unminified-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const ByteEfficiencyAudit = require('./byte-efficiency-audit.js');
const UnusedCSSRules = require('./unused-css-rules.js');
const i18n = require('../../lib/i18n/i18n.js');
const computeTokenLength = require('../../lib/minification-estimator').computeCSSTokenLength;
const computeTokenLength = require('../../lib/minification-estimator.js').computeCSSTokenLength;

const UIStrings = {
/** Imperative title of a Lighthouse audit that tells the user to minify (remove whitespace) the page's CSS code. This is displayed in a list of audit titles that Lighthouse generates. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const ByteEfficiencyAudit = require('./byte-efficiency-audit.js');
const i18n = require('../../lib/i18n/i18n.js');
const computeTokenLength = require('../../lib/minification-estimator').computeJSTokenLength;
const computeTokenLength = require('../../lib/minification-estimator.js').computeJSTokenLength;

const UIStrings = {
/** Imperative title of a Lighthouse audit that tells the user to minify the page’s JS code to reduce file size. This is displayed in a list of audit titles that Lighthouse generates. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const parseCacheControl = require('parse-cache-control');
const Audit = require('../audit.js');
const NetworkRequest = require('../../lib/network-request.js');
const URL = require('../../lib/url-shim.js');
const linearInterpolation = require('../../lib/statistics').linearInterpolation;
const linearInterpolation = require('../../lib/statistics.js').linearInterpolation;
const i18n = require('../../lib/i18n/i18n.js');
const NetworkRecords = require('../../computed/network-records.js');

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/load-fast-enough-for-pwa.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

const isDeepEqual = require('lodash.isequal');
const Audit = require('./audit.js');
const mobileThrottling = require('../config/constants').throttling.mobileSlow4G;
const mobileThrottling = require('../config/constants.js').throttling.mobileSlow4G;
const Interactive = require('../computed/metrics/interactive.js');
const i18n = require('../lib/i18n/i18n.js');

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ function generateConfig(configJson, flags) {

lighthouse.generateConfig = generateConfig;
lighthouse.getAuditList = Runner.getAuditList;
lighthouse.traceCategories = require('./gather/driver').traceCategories;
lighthouse.traceCategories = require('./gather/driver.js').traceCategories;
lighthouse.Audit = require('./audits/audit.js');
lighthouse.Gatherer = require('./gather/gatherers/gatherer.js');
lighthouse.NetworkRecords = require('./computed/network-records.js');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const BaseNode = require('../base-node.js');
const TcpConnection = require('./tcp-connection.js');
const ConnectionPool = require('./connection-pool.js');
const DNSCache = require('./dns-cache.js');
const mobileSlow4G = require('../../../config/constants').throttling.mobileSlow4G;
const mobileSlow4G = require('../../../config/constants.js').throttling.mobileSlow4G;

/** @typedef {BaseNode.Node} Node */
/** @typedef {import('../network-node')} NetworkNode */
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const fs = require('fs');
const path = require('path');
const URL = require('./lib/url-shim.js');
const Sentry = require('./lib/sentry.js');
const generateReport = require('./report/report-generator').generateReport;
const generateReport = require('./report/report-generator.js').generateReport;
const LHError = require('./lib/lh-error.js');

/** @typedef {import('./gather/connections/connection.js')} Connection */
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/scripts/benchmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

/* eslint-disable no-console */

const ultradumbBenchmark = require('../lib/page-functions').ultradumbBenchmark;
const ultradumbBenchmark = require('../lib/page-functions.js').ultradumbBenchmark;

console.log('Running ULTRADUMB™ benchmark 10 times...');

Expand Down
47 changes: 0 additions & 47 deletions lighthouse-core/scripts/dot-js.js

This file was deleted.

6 changes: 3 additions & 3 deletions lighthouse-core/scripts/lantern/run-once.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

const fs = require('fs');
const path = require('path');
const PredictivePerf = require('../../../lighthouse-core/audits/predictive-perf.js');
const Simulator = require('../../../lighthouse-core/lib/dependency-graph/simulator/simulator.js');
const traceSaver = require('../../../lighthouse-core/lib/lantern-trace-saver.js');
const PredictivePerf = require('../../audits/predictive-perf.js');
const Simulator = require('../../lib/dependency-graph/simulator/simulator.js');
const traceSaver = require('../../lib/lantern-trace-saver.js');

if (process.argv.length !== 4) throw new Error('Usage $0 <trace file> <devtools file>');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const RenderBlockingResourcesAudit = require('../../../audits/byte-efficiency/render-blocking-resources.js'); // eslint-disable-line max-len

const mobileSlow4G = require('../../../config/constants').throttling.mobileSlow4G;
const mobileSlow4G = require('../../../config/constants.js').throttling.mobileSlow4G;
const NetworkNode = require('../../../lib/dependency-graph/network-node.js');
const CPUNode = require('../../../lib/dependency-graph/cpu-node.js');
const Simulator = require('../../../lib/dependency-graph/simulator/simulator.js');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const FastPWAAudit = require('../../audits/load-fast-enough-for-pwa.js');
const mobileSlow4GThrottling = require('../../config/constants').throttling.mobileSlow4G;
const mobileSlow4GThrottling = require('../../config/constants.js').throttling.mobileSlow4G;
const assert = require('assert');
const createTestTrace = require('../create-test-trace.js');
const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'use strict';

// NOTE: this require path does not resolve correctly.
// eslint-disable-next-line local-rules/require-file-extension
const LighthouseAudit = require('../terrible/path/come/on/audit.js');

class RequireErrorAudit extends LighthouseAudit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'use strict';

// NOTE: this require path does not resolve correctly.
// eslint-disable-next-line local-rules/require-file-extension
const LighthouseGatherer = require('../terrible/path/no/seriously/gatherer.js');

class CustomGatherer extends LighthouseGatherer {}
Expand Down
Loading

0 comments on commit df85d68

Please sign in to comment.