Hacker News Reader

Comment by marmaduke | Hacker News Reader

How well does this handle dynamic constructs? Eg if I rename a method and elsewhere have

    if hasattr(obj, ‘foo’):

does that get caught or not?

View on HN

No, it would not catch that, because it's not tracing elements through the AST. It would be possible to do some amount of tracing, but it would be impossible to catch and modify all variants of that, since Python is an extremely dynamic language. Imagine something like:

    attr = "foo" if conditionA else "bar"
    obj = A() if conditionB else B()
    if hasattr(obj, attr):
        ...

Because of this, Bowler is focused on being more practical, and leans on the expectation that an engineer will be in the loop and validate the resulting diff, run unit tests, etc.

I apologize but I'm really having trouble understanding what the problem you are referring too. In python syntax that would be written as -

    if ConditionA:
        attr = "foo"
    else:
        attr = "bar"

    if ConditionB:
        obj = A()
    else:
        obj = B()

    if hasattr(attr, obj):
        do_magic()

I haven't tested Bowler yet but I would think something that refactors code would handle that pretty easily?

A() and B() could return two different classes that both support foo and bar methods. If your refactor is to rename one of those methods for one of the classes this code breaks and it's not trivial to update automatically, is it?

in general, given this is python, A and B could do something completely foul like request data from the internets and `eval(...)` it then return the result, or redefine `True, False = False, True` or redefine

  hasattr = lambda *args: False`

or something equally insane and difficult to statically analyse

> Eg if I rename a method and elsewhere have `if hasattr(obj, ‘foo’):`

Further, even in the absence of insane dynamic code, it probably isn't possible to automatically make a correct decision without knowing the intent of the original code.

For example, maybe we're renaming the `foo` method of `Fizz` class to `barr`, and currently the program is written such that `is hasattr(obj, 'foo'):` only happens to be called on `Fizz` instances. Is it the case that the programmer only ever intended that code to be called on `Fizz` instances, or did the programmer intend it should operate on all types that have a `foo` attribute, including old-style `Fizz` instances and new types that might be encountered in future? In the former case, we should rewrite the logic to `hasattr(obj, 'barr')`, in the latter case perhaps we should leave it unchanged, so it will no longer trigger after the refactor.

i do agree it should be possible to automatically flag this as "here be something interesting to consider! please intervene and make a manual decision!"

I think these sorts of cases of obfuscated code are outside the scope of refactoring tools. No one who’s looking to use a refactoring tool has to write or read code that uses eval or redefines True.

> I would think something that refactors code would handle that pretty easily?

Nope. Something that refactors code could potentially identify that as an ambiguous case reliably, but it's impossible to statically refactor code of that sort in the general case. Without external/runtime behavior verification/specification, that is.

>In python syntax that would be ...

That was Python syntax.

See below

  Python 3.6.6 (tags/v3.6.6:4cf1f54eb7, Jul  6 2018, 15:35:19) 
  [GCC 4.8.4] on linux
  Type "help", "copyright", "credits" or "license" for more information.
  >>> conditionA = True
  >>> attr = "foo" if conditionA else "bar"
  >>> print(attr)
  foo
  >>> conditionA = False
  >>> attr = "foo" if conditionA else "bar"
  >>> print(attr)
  bar
  >>>

Reply on news.ycombinator.com