Fix solidity error parser. Fixes #4896.

[bogdan]

Fixes #4896 .

[stale]

This pull request has been automatically marked as stale because it has not been updated recently. Make sure to write tests and document your changes. See :help ale-dev for information on writing tests.
If your pull request is good to merge, bother w0rp or another maintainer again, and get them to merge it.

[rymdbar]

Thanks for the fix @bogdan !

Unfortunately some test cases are failing. Did you by any chance have had a look at why?

[bogdan]

Yes, it looks like it breaks something. While I was investigating, I found that solc allows json output. I believe that would be far better solution than trying to part text input that is designed to be very human readable not very machine readable.

 $ solc \
  --base-path . \
  --include-path node_modules \
  --standard-json <<EOF | jq
{
  "language": "Solidity",
  "sources": {
    "contracts/ArtistCollection.sol": {
      "urls": ["contracts/ArtistCollection.sol"]
    }
  }
}
EOF
{
  "errors": [
    {
      "component": "general",
      "errorCode": "7920",
      "formattedMessage": "DeclarationError: Identifier not found or not unique.\n  --> contracts/BaseCollection.sol:19:37:\n   |\n19 | abstract contract BaseCollection is ER721BurnableUpgradeable, MinterCheckers {\n   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^\n\n",
      "message": "Identifier not found or not unique.",
      "severity": "error",
      "sourceLocation": {
        "end": 1320,
        "file": "contracts/BaseCollection.sol",
        "start": 1296
      },
      "type": "DeclarationError"
    }
  ],
  "sources": {}
}

[rymdbar]

So what is missing is updating GetCommand() to include the format flag and rewrite the json to l:output?

Both vim and neovim has a json_decode() function which many existing ALE linter configurations make use of. I don't have convenient access, or experience with, solidity. How about something like this?

diff --git a/ale_linters/solidity/solc.vim b/ale_linters/solidity/solc.vim
index a9479f14..0d5668c1 100644
--- a/ale_linters/solidity/solc.vim
+++ b/ale_linters/solidity/solc.vim
@@ -5,34 +5,31 @@ call ale#Set('solidity_solc_executable', 'solc')
 call ale#Set('solidity_solc_options', '')
 
 function! ale_linters#solidity#solc#Handle(buffer, lines) abort
-    " Matches patterns like the following:
-    " Error: Expected ';' but got '('
-    "    --> /path/to/file/file.sol:1:10:)
-    let l:buffer_name = bufname(a:buffer)
-    let l:pattern = '\v(Error|Warning|Note): (.*)$'
-    let l:line_and_column_pattern = '\v--\> (.*\.sol):(\d+):(\d+):'
+    try
+        let l:data = json_decode(join(a:lines, ''))
+    catch
+        return []
+    endtry
+
+    if empty(l:data)
+        return []
+    endif
+
     let l:output = []
-    let l:type = 'Note'
-    let l:text = ''
-
-    for l:line in a:lines
-        let l:match = matchlist(l:line, l:pattern)
-
-        if len(l:match) == 0
-            let l:match = matchlist(l:line, l:line_and_column_pattern)
-
-            if len(l:match) > 0 && l:type isnot# 'Note' && l:match[1] is# l:buffer_name
-                call add(l:output, {
-                \   'lnum': l:match[2] + 0,
-                \   'col': l:match[3] + 0,
-                \   'text': l:text,
-                \   'type': l:type is? 'Error' ? 'E' : 'W',
-                \})
-            endif
-        else
-            let l:type = l:match[1]
-            let l:text = l:match[2]
-        endif
+
+    for l:type in ['errors']
+        for l:object in l:data[l:type]
+            let l:line = get(l:object['sourceLocation'], 'start', -1)
+            let l:message = l:object['message']
+            let l:detail = l:object['formattedMessage']
+            " \   'lnum': l:line,
+            call add(l:output, {
+            \   'lnum': l:line,
+            \   'text': l:message,
+            \   'type': 'E',
+            \   'detail': l:detail,
+            \})
+        endfor
     endfor
 
     return l:output
@@ -41,7 +38,8 @@ endfunction
 function! ale_linters#solidity#solc#GetCommand(buffer) abort
     let l:executable = ale#Var(a:buffer, 'solidity_solc_executable')
 
-    return l:executable . ale#Pad(ale#Var(a:buffer, 'solidity_solc_options')) . ' %s'
+    return l:executable . ' --standard-json' .
+    \   ale#Pad(ale#Var(a:buffer, 'solidity_solc_options')) . ' %s'
 endfunction
 
 call ale#linter#Define('solidity', {

It clearly needs a few iterations to get all the details right, but parsing json is indeed to be preferred over the output intended for human eyes.

[bogdan]

@rymdbar here is the problem: argotorg/solidity#16107. I am open for advice on how to solve it without the patch to solidity itself.

[rymdbar]

@rymdbar here is the problem: argotorg/solidity#16107.

Ah! That indeed makes things a bit trickier. I did notice the "line" numbers were unrealistically high, but failed to realize why.

While there is a byte2line() built-in function, it is unfortunately rather useless as it only works with the current buffer. One could perhaps use something like the following to convert from offset to line number:

function! OffsetToLine(buffer, offset)
    if getbufvar(a:buffer, '&fileformat') == 'dos'
        let eol = 2
    else
        let eol = 1
    endif
    let l:lines = getbufline(a:buffer, 1, '$')
    let l:position = 0
    for line in range(1, len(l:lines))
        let l:position += len(l:lines[line-1]) + eol
        if l:position >= a:offset
            return l:line
        endif
    endfor
    return -1
endfunction

I don't know how the performance of looking up like this might be, nor what other problems using it might lead to.