Plugin-php: Document how to ignore sections of files, or whole files

[NateRadebaugh]

[alexander-akait]

@NateRadebaugh Should be works https://prettier.io/docs/en/ignore.html. If some code will be change with comment(s) please create new issue with examples. Thanks!

[NateRadebaugh]

That's for whole files, but how do we ignore a section of a file? I have some graphql in my PHP I don't want to be collapsed.

[alexander-akait]

@NateRadebaugh Can you provide example? Should works:
```

$var = 1;

[sseypt]

@evilebottnawi What am I missing here?

Input:

<?php

// <!-- prettier-ignore-start -->
$var1   =   1;
// <!-- prettier-ignore-end -->

/* <!-- prettier-ignore-start --> */
$var2   =   1;
/* <!-- prettier-ignore-end --> */

// prettier-ignore
$var3   =   1;

/* prettier-ignore */
$var4   =   1;

Output:

<?php
// <!-- prettier-ignore-start -->
$var1 = 1;
// <!-- prettier-ignore-end -->
/* <!-- prettier-ignore-start --> */
$var2 = 1;
/* <!-- prettier-ignore-end --> */

// prettier-ignore
$var3 = 1;

/* prettier-ignore */
$var4 = 1;

Nothing is ignored.

[nikolasmatt]

To respect ignores we need the printer to export a hasPrettierIgnore function like here

So far only built-in language that supports start-to-end ignore is markdown but it's implementation is not too complicated

We need to decide out which markers we want to use for php. Using /* <!-- prettier-ignore-start/end --> */ seems like an overkill, maybe just /* prettier-ignore-start/end */ ?

Additionally, personal opinion would be to add an @ to the front of these similar to PHP_CodeSniffer tags, but there is something to say for consistency with prettier.

Lastly, I have something almost there in this branch but I'm facing an issue where it drops the semicolon at the end of the node. I've spend a couple of hours and I'm not getting anywhere. Would much appreciate a second pair of eyes!
Seems to me like options.locEnd(node) points to the character before the ; here

[mgrip]

Awesome thanks @nikolasmatt! 🎉

My vote would be for the regular php comments as well. Looks like the way you have it implemented either // @prettier-ignore or // prettier-ignore will work. And where exactly is the problem with the semicolon? The test output looks good from what I can see

[nikolasmatt]

tests in my branch don't pass

input:

// prettier-ignore
$var1   =   1;

current output

// prettier-ignore
$var1   =   1

[mgrip]

Gotchya - is that the only failing one?

The locEnd is behaving as expected because the semicolon isn't part of the assign node. Normally when the node gets run through print we see if a semicolon should be added for that node type here: https://github.com/prettier/plugin-php/blob/master/src/printer.js#L170, but because this doesn't go through print that doesn't happen. Im not sure how this works for JS...

[nikolasmatt]

All the ones that end with a ; fail. The function one passes because it doesn't have a semicolon in the end.
In js the equivalent node ("type": "VariableDeclaration") includes the semicolon but doesn't include it in it's children ("declarations").

Test script I used:

"use strict";
console.log(
  JSON.stringify(
    require("@babel/parser").parse(
      "// prettier-ignore\nconst a = 1;",
      { sourceType: "module"}
    )
  )
);

output (paste it on a json prettifier):

{"type":"File","start":0,"end":31,"loc":{"start":{"line":1,"column":0},"end":{"line":2,"column":12}},"program":{"type":"Program","start":0,"end":31,"loc":{"start":{"line":1,"column":0},"end":{"line":2,"column":12}},"sourceType":"module","interpreter":null,"body":[{"type":"VariableDeclaration","start":19,"end":31,"loc":{"start":{"line":2,"column":0},"end":{"line":2,"column":12}},"declarations":[{"type":"VariableDeclarator","start":25,"end":30,"loc":{"start":{"line":2,"column":6},"end":{"line":2,"column":11}},"id":{"type":"Identifier","start":25,"end":26,"loc":{"start":{"line":2,"column":6},"end":{"line":2,"column":7},"identifierName":"a"},"name":"a"},"init":{"type":"NumericLiteral","start":29,"end":30,"loc":{"start":{"line":2,"column":10},"end":{"line":2,"column":11}},"extra":{"rawValue":1,"raw":"1"},"value":1}}],"kind":"const","leadingComments":[{"type":"CommentLine","value":" prettier-ignore","start":0,"end":18,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":18}}}]}],"directives":[]},"comments":[{"type":"CommentLine","value":" prettier-ignore","start":0,"end":18,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":18}}}]}

[mgrip]

I'm unsure if thats something the parser should be doing or not - @evilebottnawi @czosel @ichiriac do you guys have opinions?

Either way to test it out we could do something hacky here: https://github.com/prettier/plugin-php/blob/master/src/index.js#L44 by checking if that node type should have a semicolon, and if yes then adding 1 to locEnd. Would be good to see if it breaks any other tests as well

[ichiriac]

Hi @mgrip, it's not working on too many cases, the +1 on locEnd does not work it you have extra spaces before the ;, and node types does not always acts as expected : callfn($a = $b = 123);

The consumption of the ; is done at the token level and I'm agree that it should be included into the parsed statement (I guess it will be complicated to be implemented)

Another issue it that as the ; is only required to validate syntax, but not really parsed, the following can not be ignored actually : $a = 1;;; - I've missed the noop / emptystatement AST node.

[nikolasmatt]

$a = 1;;; can be ignored since the comment applies only to the next node, and ; marks the end of a node. Prettier does the same:
https://prettier.io/playground/#N4Igxg9gdgLgprEAuEB6VACADgJzjGASzhwFpCBzKCPAHSkigGcYMBDDTjAXi4wEY+GANwZGLPgCMeQgbJH1xrMDK6DOo9NjwFiZStToNoEgCZ9earsPpbc+IiXJUacRSdZuonXvVl+1enobKDsdR30XIyUMADMLPnVrYRT3ZlYKIUs1TQAqbQc9Z0M4DFzUNIkACyyhfjyC3ScDVzLMGMJaxOEQABoQCCwiE2RQNhwcCAB3AAVxhCZkEDYANwhCUz6QSRw2MABrfABlLD3CKApkGBwAVzh+86YSGBndigBbNmRYtgAbJ-6ACsmAAPABCuwOxzY7zgABlznBvn8ASBTjgnjglpI2JIAJ6-aBbXDnGAAdQ2MCqyAAHAAGfq4CBPMm7LBLeyYlZI-p4ACON0IeFebA+XyQP3+9xAT3ehCut2lTHOFF+cAAijcIPBkVL+jBcRTTFTkAAmfW7Qi-FUAYQg70+S2oUB5IBuTwAKrjFhKUXAAL7+oA

Maybe instead of +1 on locEnd we use find next non-space, non-break character and if it's ; use it's offset, otherwise keep as is?

[ichiriac]

Hi @nikolasmatt,

It could work as the ; terminal is in principle not used anywhere else.

It could be a quick fix, but it will also be fixed in the parser as it's a matter of consistency : sometimes the ; is included into the statement node, and sometimes not.

I've opened an issue about this, you can try with this commit, it should fix this issue : https://github.com/glayzzle/php-parser/commit/34ed26f07a0336a7f54a405fcd844dc713675876

[nikolasmatt]

Thank you @ichiriac!

Like you said I tried it out with your commit, and it fixes this issue. Yay! 🍾

Unfortunately it breaks 8 other tests 😞nikolasmatt/php-plugin@3e1aa24

It does look like a step forward though. So maybe the order of resolution is:

1. Fix this in a new version of the parser.
2. Update dependency version and resolve conflicts.
3. Pull fix from my branch

[ichiriac]

Hi @NateRadebaugh, I've made some additional fixes because before it was also including ?> tokens. Can you check this version : https://github.com/glayzzle/php-parser/commit/a6fe467a24dd98aff3102234107dab2fa2986743 - if it's OK it will be included in the future release.

[nikolasmatt]

@ichiriac, I merged in the most recent prettier/php-plugin changes and pointed to glayzzle/php-parser@a6fe467

Looks the same as the last commit. 2 suites (8 tests) are failing. Same ones as before.
Let me know if you need anything else.

Looking forward to the release!

my branch for reference https://github.com/nikolasmatt/plugin-php/tree/issue-246

[ichiriac]

ok, it was a bug on trailling comas on arrays, with this version it should be OK but it also introduce changes on nodes, take a look at this issue : https://github.com/glayzzle/php-parser/issues/83#issuecomment-408622834

[mgrip]

@ichiriac would you mind releasing a new version of php-parser so we can start working on this? thanks!

[vielhuber]

Is this already implemented?

I add

<?php
/* prettier-ignore */

at the top of my file, but the file also does get transformed.

[loilo]

/* prettier-ignore */ will only ignore the next node. To ignore your whole file, you need start and end markers:

<?php
// prettier-ignore-start



// prettier-ignore-end

See this playground link.

(That said, it would probably be reasonable if the end marker was not needed explicitely.)

[vielhuber]

Awesome, thank you.

[vielhuber]

@Loilo:

This seems to kill a lot of my files:

<?php
// prettier-ignore-start
foo();
?>
    <span>OK</span>
<?php
bar();
// prettier-ignore-end

output:

<?php
// prettier-ignore-start
(); ?>
    <span>OK</span>
<?php ();
// prettier-ignore-end

Shall I make a new bug report?

[loilo]

Oh dear.
Yes, please. 🙈