Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
willdurand committed May 25, 2020
1 parent 0c74cdc commit 1728d9d
Show file tree
Hide file tree
Showing 15 changed files with 93 additions and 219 deletions.
2 changes: 1 addition & 1 deletion src/rules/javascript/content-scripts-file-absent.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
CONTENT_SCRIPT_EMPTY,
} from 'messages/javascript';

export default {
module.exports = {
create(context) {
const existingFiles = Object.keys(context.settings.existingFiles || {}).map(
(fileName) => {
Expand Down
6 changes: 4 additions & 2 deletions src/rules/javascript/deprecated-entities.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { NO_DOCUMENT_WRITE } from 'messages';
import { getNodeReference } from 'utils';

export const DEPRECATED_ENTITIES = [
const DEPRECATED_ENTITIES = [
{
error: NO_DOCUMENT_WRITE,
object: 'document',
property: 'write',
},
];

export default {
module.exports = {
DEPRECATED_ENTITIES,

create(context) {
return {
// eslint-disable-next-line consistent-return
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/event-listener-fourth.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { EVENT_LISTENER_FOURTH } from 'messages/javascript';
import { getNodeReference } from 'utils';

export default {
module.exports = {
create(context) {
return {
// eslint-disable-next-line consistent-return
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/global-require-arg.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { getVariable } from 'utils';
* This rule will detect a global passed to `require()` as the first arg
*
*/
export default {
module.exports = {
create(context) {
return {
// eslint-disable-next-line consistent-return
Expand Down
23 changes: 0 additions & 23 deletions src/rules/javascript/index.js

This file was deleted.

2 changes: 1 addition & 1 deletion src/rules/javascript/opendialog-nonlit-uri.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { OPENDIALOG_NONLIT_URI } from 'messages';

export default {
module.exports = {
create(context) {
return {
// eslint-disable-next-line consistent-return
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/opendialog-remote-uri.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { isLocalUrl } from 'utils';
import { OPENDIALOG_REMOTE_URI } from 'messages';

export default {
module.exports = {
create(context) {
return {
// eslint-disable-next-line consistent-return
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/webextension-api-compat-android.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ANDROID_INCOMPATIBLE_API } from 'messages/javascript';
import { createCompatibilityRule } from 'utils';
import { hasBrowserApi } from 'schema/browser-apis';

export default {
module.exports = {
create(context) {
return createCompatibilityRule(
'firefox_android',
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/webextension-api-compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { INCOMPATIBLE_API } from 'messages/javascript';
import { createCompatibilityRule } from 'utils';
import { hasBrowserApi } from 'schema/browser-apis';

export default {
module.exports = {
create(context) {
return createCompatibilityRule(
'firefox',
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/webextension-api.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { isTemporaryApi } from 'schema/browser-apis';
import { apiToMessage, isBrowserNamespace } from 'utils';

export default {
module.exports = {
create(context) {
return {
// eslint-disable-next-line consistent-return
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/webextension-deprecated-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { DEPRECATED_JAVASCRIPT_APIS } from 'const';
import { isDeprecatedApi, hasBrowserApi } from 'schema/browser-apis';
import { isBrowserNamespace } from 'utils';

export default {
module.exports = {
create(context) {
return {
MemberExpression(node) {
Expand Down
2 changes: 1 addition & 1 deletion src/rules/javascript/webextension-unsupported-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { UNSUPPORTED_API } from 'messages/javascript';
import { hasBrowserApi } from 'schema/browser-apis';
import { isBrowserNamespace } from 'utils';

export default {
module.exports = {
create(context) {
return {
MemberExpression(node) {
Expand Down
167 changes: 74 additions & 93 deletions src/scanners/javascript.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,17 @@
import ESLint from 'eslint';
import path from 'path';

import ESLint from 'eslint';
import { oneLine } from 'common-tags';
import espree from 'espree';
import vk from 'eslint-visitor-keys';

import { ESLINT_RULE_MAPPING, ESLINT_TYPES } from 'const';
import * as messages from 'messages';
import { rules } from 'rules/javascript';
import { ensureFilenameExists } from 'utils';

const ECMA_VERSION = 2019;

export function excludeRules(excludeFrom = {}, excludeWhat = []) {
return Object.keys(excludeFrom).reduce((result, ruleName) => {
if (excludeWhat.includes(ruleName)) return result;
return {
...result,
[ruleName]: excludeFrom[ruleName],
};
}, {});
}

export default class JavaScriptScanner {
_defaultRules = rules;

disabledRules = [];

constructor(code, filename, options = {}) {
Expand Down Expand Up @@ -53,25 +41,20 @@ export default class JavaScriptScanner {

async scan(
_ESLint = ESLint,
{
_rules = this._defaultRules,
_ruleMapping = ESLINT_RULE_MAPPING,
_messages = messages,
} = {}
{ _ruleMapping = ESLINT_RULE_MAPPING, _messages = messages } = {}
) {
this._ESLint = ESLint;
this.sourceType = this.detectSourceType(this.filename);

const configDefaults = {
envs: {
es6: true,
webextensions: true,
browser: true,
},
settings: {
addonMetadata: this.options.addonMetadata,
existingFiles: this.options.existingFiles,
},
const rules = {};
Object.keys(_ruleMapping).forEach((ruleName) => {
if (!this.disabledRules.includes(ruleName)) {
rules[ruleName] = _ruleMapping[ruleName];
this._rulesProcessed++;
}
});

const eslintConfig = {
envs: ['es6', 'webextensions', 'browser'],
// It's the default but also shouldn't change since we're using
// espree to parse javascript files below manually to figure out
// if they're modules or not
Expand All @@ -80,91 +63,89 @@ export default class JavaScriptScanner {
ecmaVersion: ECMA_VERSION,
sourceType: this.sourceType,
},
rules: _ruleMapping,
plugins: ['no-unsafe-innerhtml'],
rules,
rulePaths: [path.join(__dirname, '..', 'rules', 'javascript')],
plugins: ['no-unsanitized'],
allowInlineConfig: false,

// Disable ignore-mode and overwrite eslint default ignore patterns
// so an add-on's bower and node module folders are included in
// the scan. See: https://github.com/mozilla/addons-linter/issues/1288
ignore: false,
patterns: ['!bower_components/*', '!node_modules/*'],
// Also, don't ignore dotfiles in scans.
dotfiles: true,

filename: this.filename,

// Avoid loading the addons-linter .eslintrc file
useEslintrc: false,
};

const linter = new _ESLint.Linter();

const rulesAfterExclusion = excludeRules(_rules, this.disabledRules);

Object.keys(rulesAfterExclusion).forEach((name) => {
this._rulesProcessed++;
linter.defineRule(name, rulesAfterExclusion[name]);
});
baseConfig: {
settings: {
addonMetadata: this.options.addonMetadata,
existingFiles: this.options.existingFiles,
},
},
};

const results = linter.verify(this.code, configDefaults, this.filename);
const cli = new _ESLint.CLIEngine(eslintConfig);
const { results } = cli.executeOnText(this.code, this.filename, true);

// eslint prepends the filename with the current working directory,
// strip that out.
this.scannedFiles.push(this.filename);

results.forEach((message) => {
// Fatal error messages (like SyntaxErrors) are a bit different, we
// need to handle them specially.
if (message.fatal === true) {
// eslint-disable-next-line no-param-reassign
message.message = _messages.JS_SYNTAX_ERROR.code;
}
results.forEach((result) => {
result.messages.forEach((message) => {
// Fatal error messages (like SyntaxErrors) are a bit different, we
// need to handle them specially.
if (message.fatal === true) {
// eslint-disable-next-line no-param-reassign
message.message = _messages.JS_SYNTAX_ERROR.code;
}

if (typeof message.message === 'undefined') {
throw new Error(
oneLine`JS rules must pass a valid message as
if (typeof message.message === 'undefined') {
throw new Error(
oneLine`JS rules must pass a valid message as
the second argument to context.report()`
);
}
);
}

// Fallback to looking up the message object by the message
let code = message.message;
let shortDescription;
let description;

// Support 3rd party eslint rules that don't have our internal
// message structure and allow us to optionally overwrite
// their `message` and `description`.
if (Object.prototype.hasOwnProperty.call(_messages, code)) {
({ message: shortDescription, description } = _messages[code]);
} else if (
Object.prototype.hasOwnProperty.call(
messages.ESLINT_OVERWRITE_MESSAGE,
message.ruleId
)
) {
const overwrites = messages.ESLINT_OVERWRITE_MESSAGE[message.ruleId];
shortDescription = overwrites.message || message.message;
description = overwrites.description || message.description;

if (overwrites.code) {
({ code } = overwrites);
// Fallback to looking up the message object by the message
let code = message.message;
let shortDescription;
let description;

// Support 3rd party eslint rules that don't have our internal
// message structure and allow us to optionally overwrite
// their `message` and `description`.
if (Object.prototype.hasOwnProperty.call(_messages, code)) {
({ message: shortDescription, description } = _messages[code]);
} else if (
Object.prototype.hasOwnProperty.call(
messages.ESLINT_OVERWRITE_MESSAGE,
message.ruleId
)
) {
const overwrites = messages.ESLINT_OVERWRITE_MESSAGE[message.ruleId];
shortDescription = overwrites.message || message.message;
description = overwrites.description || message.description;

if (overwrites.code) {
code = overwrites.code;
}
} else {
shortDescription = code;
description = null;
}
} else {
shortDescription = code;
description = null;
}

this.linterMessages.push({
code,
column: message.column,
description,
file: this.filename,
line: message.line,
message: shortDescription,
sourceCode: message.source,
type: ESLINT_TYPES[message.severity],
this.linterMessages.push({
code,
column: message.column,
description,
file: this.filename,
line: message.line,
message: shortDescription,
sourceCode: message.source,
type: ESLINT_TYPES[message.severity],
});
});
});

Expand Down
12 changes: 1 addition & 11 deletions tests/unit/rules/javascript/test.eslintRulesObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,11 @@ import path from 'path';
import { existsSync, readdirSync } from 'fs';

import { ESLINT_RULE_MAPPING } from 'const';
import * as jsRules from 'rules/javascript';
import { ignorePrivateFunctions } from 'utils';

describe('Eslint rules object', () => {
it('should have keys that match the file names', () => {
Object.keys(ignorePrivateFunctions(jsRules)).forEach((jsRule) => {
const jsFilePath = path.join('src/rules/javascript/', `${jsRule}.js`);
expect(existsSync(jsFilePath)).toBeTruthy();
});
});

it('should have files that match the keys', () => {
const files = readdirSync('src/rules/javascript').filter(
(fileName) => fileName !== 'index.js'
);
const files = readdirSync('src/rules/javascript');
files.forEach((fileName) => {
expect(
Object.prototype.hasOwnProperty.call(
Expand Down
Loading

0 comments on commit 1728d9d

Please sign in to comment.