ionide / ionide-vscode-fsharp

VS Code plugin for F# development
http://ionide.io
MIT License
865 stars 280 forks source link

Show Fable errors in IDE #412

Closed alfonsogarciacaro closed 7 years ago

alfonsogarciacaro commented 7 years ago

After merging https://github.com/fable-compiler/Fable/pull/806 now Fable can output multiple errors

capture

In the image, only the start line and column are shown to make the error message compatible with VS format but it's easy to add the end line and column as well.

It'd be very nice if Ionide could mark these errors in the IDE. What would be needed to that? I guess we should add a command to start Fable watching and then a regular expression to match the error messages in the process output. I can try to PR it if you could provide some guidance :+1:

Krzysztof-Cieslak commented 7 years ago

That should be pretty easy to add on VSCode side.

The interesting question is how user workflow should look like:

  1. Should user manually start it or should we detect that it's Fable project?
  2. Should it be normal "compilation" process using all configuration user has defined or additional one outputting files to some temporary dir. Or maybe even better, create flag in Fable that let us to run error checking without outputting any code?

It's obviously your decision but maybe we should create "silent, error checking mode" in Fable that only checks the errors, and prints them to console, without printing all other stuff and without outputing js files?

alfonsogarciacaro commented 7 years ago

That could be interesting, but the main problem is Fable doesn't have a light mode like the F# compiler just to check for errors, so we'd have to create the full AST everytime (well, we could save the Babel part, but that's usually pretty fast). Because devs usually run Fable in watch mode, it could be a waste of computing resources to have two Fable processes running to do basically the same thing...

For now, I'd just let users start Fable in watch mode through a command in VS Code (the convention is to use dotnet fable npm-run start but I'd allow also custom commands), capture the error messages in the output and display them in the IDE.

Krzysztof-Cieslak commented 7 years ago

I actually wonder if we need to do anything.... Have you tried using $mscompile as problem matcher for task defined in tasks.json ?

alfonsogarciacaro commented 7 years ago

I just did, but unfortunately with little success.

capture3

Krzysztof-Cieslak commented 7 years ago

OK, that's bit unfortunate but we can solve it.

In last VSCode update they've added ability to define problem matchers on plugin level. Do you think it would be enough to provide Fable problem matcher (so people can do "problemMatcher": "$fable" ) and just recommend using tasks, or do we need custom commands etc? We can also provide snippet for Fable-default task.

alfonsogarciacaro commented 7 years ago

Thanks @Krzysztof-Cieslak. Yes, that could probably be enough. I gave it a try and got it working using the following task configuration:

{
  "version": "0.1.0",
  "command": "dotnet",
  "isShellCommand": true,
  "args": [],
  "tasks": [
    {
      "taskName": "fable",
      "suppressTaskName": true,
      "args": [ "build/fable/dotnet-fable.dll", "npm-run", "webpack", "--args", "-w" ],
      "isBuildCommand": true,
      "showOutput": "always",
      "problemMatcher": {
          "owner": "fsharp",
          "fileLocation": "absolute",
          "pattern": {
              "regexp": "^(.*)\\((\\d+),(\\d+),(\\d+),(\\d+)\\)\\s*:\\s*(warning|error)\\s+(\\w+)\\s*:\\s*(.*)$",
              "file": 1,
              "line": 2,
              "column": 3,
              "endLine": 4,
              "endColumn": 5,
              "severity": 6,
              "code": 7,
              "message": 8
          }
      }
    }
  ]
}

The main problem is it's not playing well with the watch mode :/ The VS Code task watching config requires both start and end patterns but for some reason it's tremendously difficult to get Webpack to output a message after a compilation and all errors/warnings have been printed, at least I haven't managed to do it even using plugins.

Krzysztof-Cieslak commented 7 years ago

I think custom implementation would have same problem - we need to know when last run ended and we're getting new errors

Krzysztof-Cieslak commented 7 years ago

@alfonsogarciacaro Can you hack me beginPattern regex that should be used for Fable's watch mode?

alfonsogarciacaro commented 7 years ago

For now I think we can just use "Fable loader sent", but we can also add some state to the fable-loader to try to decide when a watch compilation has started and output a more recognizable pattern. It's a bit tricky because Webpack recommends not to add state to the loaders, but we can think of a way. It's also easy to use a plugin for this.

About the end pattern, I've tried to contact Webpack and VS Code teams without success. The problem is Wepback outputs all warnings and errors after the compilation has finished, but we could output messages both in the console and as Webpack warnings. With parallel compilation and Webpack calculating the dependencies it's difficult to know what's exactly the last compiled file, but we could do some hacking (like using a timeout).

Krzysztof-Cieslak commented 7 years ago

I've played a bit with latest default Fable template (dotnet new fable) and it looks like that web pack is displaying info that it ended compilation after all warning/ errors.

And it should be possible to provide problem matched for it, will take a look at it tomorrow

Krzysztof-Cieslak commented 7 years ago
{
    // See https://go.microsoft.com/fwlink/?LinkId=733558
    // for the documentation about the tasks.json format
    "version": "0.1.0",
    "command": "dotnet",
    "runner": "terminal",
    "args": [],
    "tasks": [
        {
            "taskName": "Start",
            "args": ["fable", "npm-run", "start" ],
            "isBuildCommand": true,
            "suppressTaskName": true,
            "isBackground": true,
            "problemMatcher": {
                "owner": "fsharp",
                "fileLocation": "absolute",
                "background": {
                    "beginsPattern":{
                        "regexp": "(Parsing F# project)"
                    },
                    "endsPattern":{
                        "regexp": "(webpack:)"
                    }
                },
                "pattern": {
                    "regexp": "(.*)\\((\\d+),(\\d+),(\\d+),(\\d+)\\)\\s*:\\s*(warning|error)\\s+(\\w+)\\s*:\\s*(.*)$",
                    "file": 1,
                    "line": 2,
                    "column": 3,
                    "endLine": 4,
                    "endColumn": 5,
                    "severity": 6,
                    "code": 7,
                    "message": 8
                }
            }
        }
    ]
}

This tasks.json definition works with default Fable template... Tho I'm not really fan of it - it has some problems with handling errors after code is edited, it also doesn't clear errors if none errors are reported (WTF ?)

Krzysztof-Cieslak commented 7 years ago

fable error

Krzysztof-Cieslak commented 7 years ago

Another problem is fact that Fable also reports normal F# syntax error [which are reported anyway by FSAC] which means that it makes some errors reported multiple times. I wonder if there would be a way of marking Fable specific errors (cannot find replacement, etc) so we can match only on those.

alfonsogarciacaro commented 7 years ago

Hi @Krzysztof-Cieslak! Yes, I don't know exactly when but I also noticed that Webpack is outputting messages at the start and end of recompilation. I swear that was not the case when I reported this issue (I also tried to contact the Webpack developers in the Gitter room and SO but got no answer). I didn't try again because I was busy with Fable 1.1 release. I will do and see if I have the same issues as you.

About the FSHARP/FABLE errors, as seen here Fable adds a tag to the error message to identify the origin of the error. So you should be able to filter Fable errors by editing the Regex as follows:

(.*)\\((\\d+),(\\d+),(\\d+),(\\d+)\\)\\s*:\\s*(warning|error) FABLE:\\s*(.*)$

Thanks a lot for your help, Krzysztof! We can close this issue if you want.

Krzysztof-Cieslak commented 7 years ago

About the FSHARP/FABLE errors, as seen here Fable adds a tag to the error message to identify the origin of the error. So you should be able to filter Fable errors by editing the Regex as follows:

Sometimes I'm stupid and blind.

Thanks a lot for your help, Krzysztof! We can close this issue if you want.

I think I'll leave it open for a moment - we should be able to add problem matcher to Ionide, so people can use $fable instead of providing their own matcher everytime

Krzysztof-Cieslak commented 7 years ago

OK, latest Ionide should include Fable problem matcher.

The following tasks.json works on default template:

{
    // See https://go.microsoft.com/fwlink/?LinkId=733558
    // for the documentation about the tasks.json format
    "version": "0.1.0",
    "command": "dotnet",
    "runner": "terminal",
    "args": [],
    "tasks": [
        {
            "taskName": "Start",
            "args": ["fable", "npm-run", "start" ],
            "isBuildCommand": true,
            "suppressTaskName": true,
            "isBackground": true,
            "problemMatcher": "$fable"
        }
    ]
}

Could we add it to the template, maybe?

alfonsogarciacaro commented 7 years ago

Awesome, I'll do it! :clap: :clap: :clap:

alfonsogarciacaro commented 7 years ago

@Krzysztof-Cieslak I have a couple of suggestions for the problem matcher:

                "background": {
                    "beginsPattern":{
                        "regexp": "webpack: Compiling"
                    },
                    "endsPattern":{
                        "regexp": "webpack: (Compiled successfully|Failed to compile)"
                    }
                },

Using "Parsing F# project" for the beginsPattern is problematic, first it already changed in latest Fable version to "Parsing " and also it's coming from the Fable daemon itself. Meaning that if you are running webpack and Fable daemon in different terminals you won't see the message. ("webpack: Compiling" doesn't appear in the first compilation, not sure if that's a problem).

Also, I would call the problem matcher something like $fable-webpack because with other clients (like Rollup) you've to use other patterns.

Krzysztof-Cieslak commented 7 years ago

Fixed in 2.27.2

alfonsogarciacaro commented 7 years ago

Sorry again! When updating the template to call $fable-webpack matcher I realized I made another mistake in the Regex. I had to take the values for end line and column out of the first parens to make the path clickable from VS Code or Emacs terminal. This is the one I'm using now:

^(.*)\\((\\d+),(\\d+)\\): \\((\\d+),(\\d+)\\) (warning|error) FABLE: (.*)$

Which matches something like: /Users/alfonsogarciacaronunez/dev/Fable/src/templates/simple/Content/src/App.fs(8,12): (8,42) error FABLE: Cannot find replacement for System.Environment::get_MachineName

For now I just wrote the problem matcher directly in the template, we can update it later :+1: