Use start of identifier as start position

[Arcanemagus]

Would it be possible to have the ranges reported by this tool updated to start at the start of the offending identifier?

For example with the following code:

from example import (
    f401_unused as unused_module)


def c901_too_complex(x):
    if x > 1:
        if x > 2:
            if x > 3:
                if x > 4:
                    if x > 5:
                        if x > 6:
                            if x > 7:
                                if x > 8:
                                    if x > 9:
                                        if x > 10:
                                            if x > 11:
                                                pass


class Foo:
    def foo_c901_too_complex(x):
        if x > 1:
            if x > 2:
                if x > 3:
                    if x > 4:
                        if x > 5:
                            if x > 6:
                                if x > 7:
                                    if x > 8:
                                        if x > 9:
                                            if x > 10:
                                                if x > 11:
                                                    pass


def indent_unaligned():
    try:
        print('H501 %(self)s' % locals())
    except:  # <- H201
        pass

This module simply reports the start of the line as the problem location:

> python -m mccabe --min 10 customRange.py
5:1: 'c901_too_complex' 12
21:1: 'Foo.foo_c901_too_complex' 12

If the identifier start was reported instead, the positions would be 5:5 and 21:9 respectively. As it stands right now the column number is just plain wrong.

[sigmavirus24]

It's possible to do it that way, we just need to extract more information in our AST visiting classes to grab the column number (we presently only grab the line number).

[Arcanemagus]

(Finally getting around to reporting the bugs as you suggested here @sigmavirus24 😛)

[sigmavirus24]

I KNOW NOTHING! =P

[sigmavirus24]

So I've hacked this in, but in running mccabe against itself, I've get:

("76:4: 'PathGraph.to_dot'", 4)
("113:4: 'PathGraphingAstVisitor.visitFunctionDef'", 3)
("178:4: 'PathGraphingAstVisitor._subgraph'", 2)
("261:4: 'McCabeChecker.run'", 4)
("315:0: 'main'", 7)
("191:4: 'PathGraphingAstVisitor._subgraph_parse'", 5)
("155:4: 'PathGraphingAstVisitor.visitSimpleStatement'", 2)
("33:4: 'ASTVisitor.dispatch'", 2)
('TryExcept 13', 3)
('345:0 If', 2)
("147:4: 'PathGraphingAstVisitor.appendPathNode'", 2)
("272:0: 'get_code_complexity'", 5)
("298:0: '_read'", 5)
("109:4: 'PathGraphingAstVisitor.dispatch_list'", 2)
("29:4: 'ASTVisitor.default'", 2)
("238:4: 'McCabeChecker.add_options'", 2)

And what's specifically bothersome to me is

('TryExcept 13', 3)
('If 345', 2)

And the fact that I can find other places where similar formatting for the name is being done. Are you doing this for flake8 or for mccabe specifically because that output is far from useful.

[Arcanemagus]

The modifications in the linter-flake8 Atom package were only being applied to specific error codes, in this case C901 errors, which I believe are only from Mccabe?

If I understand #44 correctly that would be the same as how it currently will work when AtomLinter/linter-flake8#304 lands, as then it will use the default behavior (which in this case would ignore the whitespace at the start of the line, and select the first "word" as defined by the language, or in other words def).

If getting the identifier is posing as much of a problem as it appears to be, returning the correct column for the base node will at least get a meaningful column number being reported to users.

[sigmavirus24]

If getting the identifier is posing as much of a problem as it appears to be, returning the correct column for the base node will at least get a meaningful column number being reported to users.

It's not posing a problem. I haven't really much tried it. I'm just thinking that in the larger scheme of how things work with Flake8, this isn't how things are typically reported. If there's a problem with a function, the position of def is usually the column offset used. (I'm also lazy, so I didn't do much to try to find the actual label =P)

[Arcanemagus]

If that's the way things are done currently that's probably the better way to go.

[sigmavirus24]

Did I never send/merge a PR for this?

[sigmavirus24]

I did! #44