Bug 373850 - Highlighting ranges often wrong for constructors/__call__()/indexing.
Summary: Highlighting ranges often wrong for constructors/__call__()/indexing.
Status: RESOLVED FIXED
Alias: None
Product: kdev-python
Classification: Developer tools
Component: Language support (show other bugs)
Version: git master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Sven Brauch
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-18 17:34 UTC by Francis Herne
Modified: 2017-03-30 17:39 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Francis Herne 2016-12-18 17:34:05 UTC
In these cases, the magic-method's name doesn't appear in the code, so we highlight the opening paren or square-bracket.
```
mc = MyClass()  # <- `(` links to MyClass.__init__
mc[2] # <- `[` links to MyClass.__getitem__
```

In some cases, completely the wrong character is highlighted:
```
 class MyClass:
    def __call__(self):
        return 12
    def __getitem__(self, key):
        return 1.2

foo = {}
foo['aaa'] = MyClass()

bar = foo['aaa'][2] # First 'a' is highlighted instead of second '['
baz = foo['aaa'](2) # ')' is highlighted instead of '('
# Not sure why these behave differently
```

This seems particularly common after string literals.
Comment 1 Sven Brauch 2016-12-23 20:34:54 UTC
This is a quite frequent issue, and should definitely be fixed before 5.1 final.
Comment 2 Francis Herne 2016-12-24 01:17:13 UTC
The very frequent errors were my fault; I introduced a bug in
https://cgit.kde.org/kdev-python.git/commit/?id=94ab1ee

Fixed that in
https://commits.kde.org/kdev-python/9ba8942

There are remaining glitches if:
 - The bracket doesn't immediately follow what's being called:
   `foo (arg)` <- the space is highlighted. Uncommon.

 - The expression being called/subscripted has an incorrect range:
```
class A:
    def __call__(self): return 10
aaa = [A()]

bbb = aaa[000]()  # The middle '0' is highlighted.
```

I think these have always existed, but the second type is more common after 9ba8942 because we show uses after many more subscript expressions, which are most commonly broken.
Comment 3 Sven Brauch 2016-12-24 09:51:26 UTC
Nice, looks much better now. Thanks.

You can also use CCBUG:12345 to CC a bug with a commit without closing it.
Comment 4 Francis Herne 2016-12-25 10:56:39 UTC
Git commit 2049938304a0a8b65240d551117c8c169ee60ece by Francis Herne.
Committed on 25/12/2016 at 10:56.
Pushed by flherne into branch 'master'.

Set mostly-correct endCol on numbers, single-quoted strings and subscripts.

Before, endCol of number/string literals was identical to startCol;
 endCol of subscripts was the end of their slice node. This caused
 highlighting glitches when trying to position things after them.

Unaddressed:
 - Triple-quoted (particularly multiline) strings. This gets length 2
    (inc. quotes) or does nothing, which is no worse than at present.
   Such strings are very unusual inside expressions where we care about
   the length.

 - Space between the end of a slice and the closing bracket of a
   subscript: `a = b[ 2 ]`. Recommended against by PEP-8, but happens.

This should fix most remaining examples of bug 373850.

M  +31   -1    parser/astbuilder.cpp

https://commits.kde.org/kdev-python/2049938304a0a8b65240d551117c8c169ee60ece
Comment 5 Francis Herne 2017-01-01 21:33:33 UTC
Git commit 458f64330f89187d8efd558da550974c2777904c by Francis Herne.
Committed on 01/01/2017 at 21:32.
Pushed by flherne into branch '5.1'.

Yet more range fixes.

RangeUpdateVisitor was only run after completion of RangeFixVisitor.

When RangeFixVisitor modified both a node and a non-parent ancestor of
 that node, the ranges of the intermediate nodes weren't updated and so
 the ancestor's fix didn't take the previous fix into account.
Move the functionality of RangeUpdateVisitor into RangeFixVisitor so
 changes are propagated in time.

There was a bug in the `findString` regex that prevented it working for
 empty strings. Switch to QRegularExpression (I should have used it
 originally) and use a better regex.

Correct the ranges of byte and (python36) f-string literals, in addition
 to normal strings.
The 'b'/'f' still isn't coloured, but afaik that's KSyntaxHighlighting.

Hack comprehensions, lists and tuples in a similar way to subscripts;
 far from reliable, but ok for single-line expressions following PEP-8.

M  +63   -36   parser/astbuilder.cpp

https://commits.kde.org/kdev-python/458f64330f89187d8efd558da550974c2777904c
Comment 6 Francis Herne 2017-03-30 17:39:54 UTC
This now works fine in all common cases, there's not much room for improvement unless we can get accurate range support into the parser somehow.