Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Tokenisation of psalm array types #430

Open
BladeMF opened this issue Jun 13, 2021 · 19 comments
Open

Tokenisation of psalm array types #430

BladeMF opened this issue Jun 13, 2021 · 19 comments

Comments

@BladeMF
Copy link
Contributor

BladeMF commented Jun 13, 2021

Summary

Psalm offers very widely supported syntax (by static analysis tools) for documenting array structure. Currently the tokeniser struggles with these forms:

  1. Space between the parameters
/**
* @return array<TKey, TValue>
*/
  1. Curly braces for defining object-like arrays
/**
* @return array{firstName:string, lastName:string}
*/
  1. Newline between the parameters
/**
* @return array{
*     firstName:string, 
*     lastName:string
* }
* 
*/

There's probably more cases missing. It will also be a good idea to tokenise the property name with a specific token so its name can be changed.

Motivation

Static analysis is a really powerful tool for avoiding errors, saving time and enforcing coding standards. It really sucks when the IDE is not colouring correctly the syntaxes needed for that analysis. It also provides really good visual feedback and a way of customising the colourisation of the tokens.

Describe alternatives you've considered

I am not aware of any way of customising that tokenisation.

Additional context

I am using VS Code.

Other

I am totally up for PRing this if it is OK to be contributor. I would probably need guidance with testing and setup though - I haven't contributed to a language grammar before.

@KapitanOczywisty
Copy link
Contributor

KapitanOczywisty commented Jun 13, 2021

I was avoiding using this syntax, as there is no vscode extension which is making use of these. However, I think it's a good idea to support complex types, at least these supported by more than one tool (psalm, phan, phpstan etc). Feel free to leave syntax which should be supported and I'll try to get around to this. (I could use weird edge cases for testing.)

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 13, 2021

Actually the basic stuff is really simple. In repository:

    "php_doc_types_array_object": {
      "begin": "(array)({)",
      "beginCaptures": {
        "1": {
          "name": "keyword.other.type.php"
        },
        "2": {
          "name": "punctuation.definition.type.begin.bracket.curly.phpdoc.php"
        }
      },
      "end": "(})|(?=\\*/)",
      "endCaptures": {
        "1": {
          "name": "punctuation.definition.type.end.bracket.curly.phpdoc.php"
        }
      },
      "patterns": [
        { "include": "#php_doc_types_array_object" },
        { "include": "#php_doc_types_array_single" },
        { "include": "#php_doc_types" },
        { "match": "\\|", "name": "punctuation.separator.delimiter.php" }
      ]
    },

and then in "php_doc" add one more include.

{ "include": "#php_doc_types_array_object" },

Is it not OK to contribute to the project?

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 13, 2021

Oh, and I use phpactor running phpstan as an extension on file save in VS Code.

@KapitanOczywisty
Copy link
Contributor

Feel free to send PR, just add some testing if possible. I have simple script to generate test cases in Atom's devtools: File > Init script... and add following content, then reload Atom window.

// source: https://gist.github.com/KapitanOczywisty/ae65eb9809d847d00752896968339730
function specBuild(a){const b=a=>a.replace(/\\/g,"\\\\");a=a.replace(/^\s*\n|\n\s*$/g,"");let c=atom.grammars.grammarForScopeName("source.php").tokenizeLines(a),d=`it 'should tokenize ... correctly', ->\n  `;for(let e in(a.match(/\n/g)||[]).length&&(a=`\n    ${a.replace(/\n/g,"\n    ")}\n  `),d+=1<c.length?`lines = grammar.tokenizeLines '''${b(a)}'''\n\n`:`{tokens} = grammar.tokenizeLine '${b(a).replace(/'/g,"\\'")}'\n\n`,c)for(let a in c[e]){let f=JSON.stringify(c[e][a].scopes).replace(/","/g,"', '").replace(/"/g,"'"),g=1<c.length?`lines[${e}][${a}]`:`tokens[${a}]`;d+=`  expect(${g}).toEqual value: '${b(c[e][a].value).replace(/'/g,"\\'")}', scopes: ${f}\n`}return`\n${d}\n`}function specPretty(a){let b=function(a){return a.replace(/\\/g,"\\\\")},c=atom.grammars.grammarForScopeName("source.php"),d=c.tokenizeLine(a).tokens,e=`Test code: \`${b(a)}\`\n\n`;for(let c in d){let a=JSON.stringify(d[c].scopes);a=a.replace(/","/g,"\" \"").replace(/"/g,"`").replace(/\[|\]/g,""),e+=`${c}: \`${b(d[c].value)}\` scopes: ${a}\n`}return e}

window.specBuild = specBuild;
window.specPretty = specPretty;
console.log('init.js loaded');

Usage:

// single line:
specBuild(`function Foo(#[ParameterAttribute] $parameter) {}`)
// multi line
specBuild(`class Foo {
  public static $bar = 'baz';
}`)

From the output you should remove not relevant tokens and add test case to spec/php-spec.coffee, then run tests via Ctrl+Shift+Y.

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 13, 2021

I am using VS Code, I have never used Atom, unfortunately. I am guessing I need to add methods starting with it in php-spec.coffee to make tests?

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 13, 2021

If it is not too much to ask, how do I run tests from the command line? Currently I've only installed "coffeelint": "^1.10.1" as it is the only dependency in package.json. Should I install anything else?
(I apologise, I am not familiar with these tests and CoffeeScript).

@KapitanOczywisty
Copy link
Contributor

I'm also using vscode, but I have atom installed just to deal with testing of this repository - it's the easiest way. You can try to replicate what CI is doing, however it's not worth a hustle in my opinion.

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 13, 2021

Will Atom deal with this out of the box?

@KapitanOczywisty
Copy link
Contributor

Yes, open language-php folder in atom. If you disable built-in language-php package and clone repository into %UserProfile%\.atom\packages\language-php, you'll be able to test changes visually (after window reload).

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 13, 2021

Thank you! I will follow your advice. It may take a while and some tries (and maybe my first PR will be rubbish), but I'll get there. Can we leave this open so I can ask questions here please?

@KapitanOczywisty
Copy link
Contributor

I'm not a member of atom team, just an active contributor, so not a problem for me. Good luck!

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 13, 2021

Thanks! Last questions:

  1. Do you use your latest grammar in VS code? I'd yes - how? That's my ultimate goal - improve the grammar and then use it.
  2. Can you recommend a tool to convert cson to JSON?

@KapitanOczywisty
Copy link
Contributor

For personal testing you can use this: https://github.com/KapitanOczywisty/vscode-php-dev You need to change these lines: https://github.com/KapitanOczywisty/vscode-php-dev/blob/master/build/update-grammar.js#L62-L63 then run update-grammar.

@BladeMF
Copy link
Contributor Author

BladeMF commented Jun 14, 2021

Thanks! I will definitely use your update grammar command. I made an extension which replaces just the grammar (rather than the whole language) since I am planning to play with stuff (I find these grammar files fascinating!).

@KapitanOczywisty
Copy link
Contributor

Oh, and I use phpactor running phpstan as an extension on file save in VS Code.

@BladeMF Do phpactor supports object-like arrays for completion, type solving etc? Or this is just for phpstan?

@BladeMF
Copy link
Contributor Author

BladeMF commented Jan 13, 2022

Currently phpactor does not support object-like arrays for completion, but I find the checks by phpstan very useful.

@KapitanOczywisty
Copy link
Contributor

I'd love to move from intelephense, but seems like phpactor is still rather basic as an IDE. I'm also using phpstan, but It's not helping that much when writing the code.

@BladeMF
Copy link
Contributor Author

BladeMF commented Jan 13, 2022

Actually, I moved from intelephense because phpactor supports refactoring (I did some of them) and has just as good completion. Also, it's open source so you can contribute the completion from doc comments :-)

Currently, for my money, that's the best PHP language server out there. I've worked long years with PHP Tools and later Intelephense. No refactoring kills me.

@BladeMF
Copy link
Contributor Author

BladeMF commented Jan 13, 2022

Also, you should try tabnine for completion, I am pleasantly surprised I must say. It turns out I use it's suggestion more often than the actual one from the language server.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants