Skip to content
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

Support prettierignore and custom processors #111

Merged
merged 3 commits into from
Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package.json

# this file doesn't exist, but we use it as a filename that should be ignored
# by prettier in the tests
ignore-me.js
50 changes: 48 additions & 2 deletions eslint-plugin-prettier.js
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ module.exports = {
const usePrettierrc =
!context.options[1] || context.options[1].usePrettierrc !== false;
const sourceCode = context.getSourceCode();
const filepath = context.getFilename();
const source = sourceCode.text;

// The pragma is only valid if it is found in a block comment at the very
Expand Down Expand Up @@ -397,19 +398,64 @@ module.exports = {
context.options[0] === 'fb'
? FB_PRETTIER_OPTIONS
: context.options[0];

const prettierRcOptions =
usePrettierrc &&
prettier.resolveConfig &&
prettier.resolveConfig.sync
? prettier.resolveConfig.sync(context.getFilename(), {
? prettier.resolveConfig.sync(filepath, {
editorconfig: true
})
: null;

// prettier.getFileInfo was added in v1.13
const prettierFileInfo =
prettier.getFileInfo && prettier.getFileInfo.sync
? prettier.getFileInfo.sync(filepath, {
ignorePath: '.prettierignore'
})
: { ignored: false, inferredParser: null };

// Skip if file is ignored using a .prettierignore file
if (prettierFileInfo.ignored) {
return;
}

const initialOptions = {};

// ESLint suppports processors that let you extract and lint JS
// fragments within a non-JS language. In the cases where prettier
// supports the same language as a processor, we want to process
// the provided source code as javascript (as ESLint provides the
// rules with fragments of JS) instead of guessing the parser
// based off the filename. Otherwise, for instance, on a .md file we
// end up trying to run prettier over a fragment of JS using the
// markdown parser, which throws an error.
// If we can't infer the parser from from the filename, either
// because no filename was provided or because there is no parser
// found for the filename, use javascript.
// This is added to the options first, so that
// prettierRcOptions and eslintPrettierOptions can still override
// the parser.
//
// `parserBlocklist` should contain the list of prettier parser
// names for file types where:
// * Prettier supports parsing the file type
// * There is an ESLint processor that extracts JavaScript snippets
// from the file type.
const parserBlocklist = [null, 'graphql', 'markdown', 'html'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list seems like it might plausibly need to expand in the future. The detailed comment above it with background is definitely appreciated, but since the situation is fairly complex and difficult to comprehend, is there a simple description we can add for what the list should contain for future reference?

For example, maybe we could add a comment stating that:

This list should contain the list of prettier parser names for file types where:

  • Prettier supports parsing the file type
  • There is an ESLint processor that extracts JavaScript snippets from the file type.

if (
parserBlocklist.indexOf(prettierFileInfo.inferredParser) !== -1
) {
initialOptions.parser = 'babylon';
}

const prettierOptions = Object.assign(
{},
initialOptions,
prettierRcOptions,
eslintPrettierOptions,
{ filepath: context.getFilename() }
{ filepath }
);

const prettierSource = prettier.format(source, prettierOptions);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"eslint-plugin-self": "^1.0.1",
"mocha": "^3.1.2",
"moment": "^2.18.1",
"prettier": "^1.10.2",
"prettier": "^1.13.0",
"semver": "^5.3.0",
"vue-eslint-parser": "^2.0.2"
},
Expand Down
27 changes: 24 additions & 3 deletions test/prettier.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@ ruleTester.run('prettier', rule, {
code: `var foo = {bar: 0};\n`,
filename: getPrettierRcJsFilename('bracket-spacing'),
options: [{ bracketSpacing: false }, { usePrettierrc: false }]
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for the parser blacklist behavior? We don't need to actually include processors, but maybe one way to do it would be to put JavaScript code in a file with the graphql extension, and assert that no parsing errors are produced (since the text should be parsed as JavaScript).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I tried doing this with including a plugin, but I couldn't get the plugin to be applied within a RuleTester. Forcing the extension is a good middle ground

// Ignores filenames in .prettierignore
{
code: `("");\n`,
filename: getPrettierRcJsFilename('single-quote', 'ignore-me.js')
},
// Sets a default parser when it can't be inferred from the file extensions
{
code: `('');\n`,
filename: getPrettierRcJsFilename('single-quote', 'dummy.qqq')
},
// Overwrites the parser for file extensions prettier would try to format
// with not the babylon parser
// In the real world, eslint-plugin-markdown would transform file contents
// into JS snippets that would get passed to ESLint
{
code: `('');\n`,
filename: getPrettierRcJsFilename('single-quote', 'dummy.md')
}
],
invalid: [
Expand Down Expand Up @@ -175,9 +193,12 @@ function loadInvalidFixture(name) {

/**
* Builds a dummy javascript file path to trick prettier into resolving a specific .prettierrc file.
* @param {string} name - Prettierrc fixture basename.
* @param {string} dir - Prettierrc fixture basename.
* @returns {string} A javascript filename relative to the .prettierrc config.
*/
function getPrettierRcJsFilename(name) {
return path.resolve(__dirname, `./prettierrc/${name}/dummy.js`);
function getPrettierRcJsFilename(dir, file) {
// Use default parameters when we drop support for node 4
file = typeof file !== 'undefined' ? file : 'dummy.js';

return path.resolve(__dirname, `./prettierrc/${dir}/${file}`);
}
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -801,9 +801,9 @@ prelude-ls@~1.1.2:
version "1.1.2"
resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.1.2.tgz#21932a549f5e52ffd9a827f570e04be62a97da54"

prettier@^1.10.2:
version "1.10.2"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.10.2.tgz#1af8356d1842276a99a5b5529c82dd9e9ad3cc93"
prettier@^1.13.0:
version "1.14.3"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.14.3.tgz#90238dd4c0684b7edce5f83b0fb7328e48bd0895"

process-nextick-args@~1.0.6:
version "1.0.7"
Expand Down