5 ms·

Rf: an experimental refactoring tool for Go

241 points by robfig 5y ago

jchw 5y ago
I hope more refactoring tools come into existence in the future. Right now, writing transforms similar to this complexity using Clang would require a fairly large amount of boilerplate code, implementing at least some AST matching code and wrapping it into a frontend action. While having this for Go is good, I hope it grows and inspires others to do similar things in other languages, the same way gofmt once did.
- azernik 5y ago
  Since you're talking about Clang, I assume you're coding in C? In that case, Coccinelle [https://coccinelle.gitlabpages.inria.fr/website/sp.html https://coccinelle.gitlabpages.inria.fr/website/sp.html] may serve your purposes, and has a long history of use in the Linux kernel to modify shared APIs.
  - jchw 5y ago
    In case of Clang (and more specifically libClang,) I am usually using it to do source-to-source translations on C++ code. In those cases, sometimes I need some fairly advanced knowledge, such as the type of an expression, to match on or perform logic on. However, this still looks quite interesting for some use cases (and I am interested in such tools.)
    - azernik 5y ago
      Ooooh ouch, yeah C++ is generally rough on static analysis tools.
- anitil 5y ago
  > AST matching code and wrapping it into a frontend action. I'm working on a an AST visualization tool and accidentally stumbled in to this rabbit hole. Send help!
omeze 5y ago
I used to joke at work that the fastest, simplest way to get promoted to a staff engineering position was to finish a 1 yr refactor. Getting existing codebases to do new things is hard, and most of enterprise software engineering is basically "rework this API (in the general sense, not web-API sense) for a new product use case". This tool seems to borrow the philosophy of Go's keeping the language simple, with the tradeoff of a more complex (but fairly well supported) ecosystem to account for missing language features. IME, Golang (some would say paradoxically due to a lack of generics) is one of the best languages to refactor because of: - Forced error handling semantics so that unhappy paths are easily enumerated - Tooling like `rdep` to quickly grok the impact of a package refactor - String templates/codegen being a first-class "blessed" part of the ecosystem (i.e. you can find tooling by the Go team as a reference when writing your own) Some would balk at things like codegen being how Go handles this, but for the iteration loops at most companies at scale (where a few tools to do codegen are written & maintained by one team and consumed by others) it works well. For paradigms where you want everyone to contribute to the tooling, it works slightly less well since the barrier of understanding the ecosystem is greater than just understanding the language. Nevertheless, it seems to be the right tradeoff for most enterprise use cases.
- thanhhaimai 5y ago
  I'm a Googler, and while that might be true at another place, unfortunately this is not true at Google. Refactoring/Tech Debt/Eng Excellence work will not get you promoted pass L4 for the majority of teams. In some team, it will get you to L5, but that's rare. Getting to L6 based on "1 yr refactor" is... simply extraordinary. I haven't seen or heard of one case in 7 years here.
  - naikrovek 5y ago
    I'm pretty sure that the comment you replied to was talking about "the general rule" and not "the exception to the rule." you don't need to call out that your specific employer is different than other employers. everyone here already knew that.
    - 5y ago
CGamesPlay 5y ago
This seems pretty cool, from looking over the documentation. Two things I wish I could do that I don't see in the documentation: - Replace an import with another import. This is used when forking a go package. - Rename a package. It's also pretty unclear from the documentation which operations are "UNIMPLEMENTED", since the label simply falls directly between two different operations. It would be nice if this were made more explicit.
- harikb 5y ago
  It would be great if go.mod file’s replace directive also supported a replace that applies to all downstream users (url to url , not just url to local path) This would allow us to maintain simple forks (ones where you are only fixing a few lines and awaiting PR approval to upstream) but still don’t want to wait for the whole cycle to finish for your own internal company dependencies. I feel this is better than changing all imports. If you are truly forking with no intention to merge back, isn’t the manual find-and-replace by IDE just fine?
  - xyzzy_plugh 5y ago
    The replace directive does allow you to use a different import path, not just a local path. Unless there's something I'm missing about your use case? https://golang.org/ref/mod#go-mod-file-replace https://golang.org/ref/mod#go-mod-file-replace
    - harikb 5y ago
      Oh I never knew! Thanks. That does simplify a lot
    - tsimionescu 5y ago
      It's important to note that it still doesn't apply to downstream users (those adding your package to their go.mod), only to those building your package directly. I think this decision makes sense, so this is not a complaint.
      - 5y ago
AYBABTME 5y ago
I think this is cool but I wonder what the expected API surface for editor integrations (with mouse-click menu selection to apply a refactor) would work. Would editors code-generate this mini scripting language? Or will there be a more abstract way of providing the same intention to the tool? If editor integrations are expected to send the script-snippets, will there be facilities for programmatically generating valid script snippets?
alert0 5y ago
I've always wanted a tool that finds duplicate code (with minor variable name differences) so I can factor it out. Often when prototyping I'll copy a function thinking it will change a lot or maybe just not knowing what will change. As the application matures I'll go back and clean these up but it shouldn't be too hard for a tool to identify.
- thegeekpirate 5y ago
  https://github.com/mibk/dupl https://github.com/mibk/dupl does a great job, and it's included in https://github.com/golangci/golangci-lint https://github.com/golangci/golangci-lint (which everyone should use, tbh)
- AlphaSite 5y ago
  IntelliJ does this.
- grok22 5y ago
  https://pmd.github.io/latest/pmd_userdocs_cpd.html https://pmd.github.io/latest/pmd_userdocs_cpd.html
konart 5y ago
Shouldn't this be part of gopls or something?
- onionisafruit 5y ago
  Maybe eventually, but not while it's experimental.
iddan 5y ago
A similar concept named "Tactics" was explored in Haskell. https://reasonablypolymorphic.com/blog/towards-tactics/ https://reasonablypolymorphic.com/blog/towards-tactics/ HN Discussion: https://news.ycombinator.com/item?id=24759649 https://news.ycombinator.com/item?id=24759649
- shirogane86x 5y ago
  Relevant: https://haskellwingman.dev/ https://haskellwingman.dev/ . it's actually possible to use it if you have the haskell language server installed, as a code action (at least i think it is, haven't personally tried it yet tho)
dorian-graph 5y ago
A similar, but general purpose, tool to this is https://comby.dev/ https://comby.dev/. I've used it before locally for some structural code changes. The author is super helpful/responsive too.
MorganGallant 5y ago
I wonder if this is some sort of precursor work for the generics release? Feels like gofmt -r on steroids, and could be eventually used to do large scale transformations to the standard library?

241 points by robfig 5y ago

jchw 5y ago

I hope more refactoring tools come into existence in the future. Right now, writing transforms similar to this complexity using Clang would require a fairly large amount of boilerplate code, implementing at least some AST matching code and wrapping it into a frontend action. While having this for Go is good, I hope it grows and inspires others to do similar things in other languages, the same way gofmt once did.

azernik 5y ago
Since you're talking about Clang, I assume you're coding in C? In that case, Coccinelle [https://coccinelle.gitlabpages.inria.fr/website/sp.html https://coccinelle.gitlabpages.inria.fr/website/sp.html] may serve your purposes, and has a long history of use in the Linux kernel to modify shared APIs.
- jchw 5y ago
  In case of Clang (and more specifically libClang,) I am usually using it to do source-to-source translations on C++ code. In those cases, sometimes I need some fairly advanced knowledge, such as the type of an expression, to match on or perform logic on. However, this still looks quite interesting for some use cases (and I am interested in such tools.)
  - azernik 5y ago
    Ooooh ouch, yeah C++ is generally rough on static analysis tools.
anitil 5y ago
> AST matching code and wrapping it into a frontend action. I'm working on a an AST visualization tool and accidentally stumbled in to this rabbit hole. Send help!

omeze 5y ago

I used to joke at work that the fastest, simplest way to get promoted to a staff engineering position was to finish a 1 yr refactor. Getting existing codebases to do new things is hard, and most of enterprise software engineering is basically "rework this API (in the general sense, not web-API sense) for a new product use case". This tool seems to borrow the philosophy of Go's keeping the language simple, with the tradeoff of a more complex (but fairly well supported) ecosystem to account for missing language features. IME, Golang (some would say paradoxically due to a lack of generics) is one of the best languages to refactor because of: - Forced error handling semantics so that unhappy paths are easily enumerated - Tooling like `rdep` to quickly grok the impact of a package refactor - String templates/codegen being a first-class "blessed" part of the ecosystem (i.e. you can find tooling by the Go team as a reference when writing your own) Some would balk at things like codegen being how Go handles this, but for the iteration loops at most companies at scale (where a few tools to do codegen are written & maintained by one team and consumed by others) it works well. For paradigms where you want everyone to contribute to the tooling, it works slightly less well since the barrier of understanding the ecosystem is greater than just understanding the language. Nevertheless, it seems to be the right tradeoff for most enterprise use cases.

thanhhaimai 5y ago
I'm a Googler, and while that might be true at another place, unfortunately this is not true at Google. Refactoring/Tech Debt/Eng Excellence work will not get you promoted pass L4 for the majority of teams. In some team, it will get you to L5, but that's rare. Getting to L6 based on "1 yr refactor" is... simply extraordinary. I haven't seen or heard of one case in 7 years here.
- naikrovek 5y ago
  I'm pretty sure that the comment you replied to was talking about "the general rule" and not "the exception to the rule." you don't need to call out that your specific employer is different than other employers. everyone here already knew that.
  - 5y ago

CGamesPlay 5y ago

This seems pretty cool, from looking over the documentation. Two things I wish I could do that I don't see in the documentation: - Replace an import with another import. This is used when forking a go package. - Rename a package. It's also pretty unclear from the documentation which operations are "UNIMPLEMENTED", since the label simply falls directly between two different operations. It would be nice if this were made more explicit.

harikb 5y ago
It would be great if go.mod file’s replace directive also supported a replace that applies to all downstream users (url to url , not just url to local path) This would allow us to maintain simple forks (ones where you are only fixing a few lines and awaiting PR approval to upstream) but still don’t want to wait for the whole cycle to finish for your own internal company dependencies. I feel this is better than changing all imports. If you are truly forking with no intention to merge back, isn’t the manual find-and-replace by IDE just fine?
- xyzzy_plugh 5y ago
  The replace directive does allow you to use a different import path, not just a local path. Unless there's something I'm missing about your use case? https://golang.org/ref/mod#go-mod-file-replace https://golang.org/ref/mod#go-mod-file-replace
  - harikb 5y ago
    Oh I never knew! Thanks. That does simplify a lot
  - tsimionescu 5y ago
    It's important to note that it still doesn't apply to downstream users (those adding your package to their go.mod), only to those building your package directly. I think this decision makes sense, so this is not a complaint.
    - 5y ago

AYBABTME 5y ago

I think this is cool but I wonder what the expected API surface for editor integrations (with mouse-click menu selection to apply a refactor) would work. Would editors code-generate this mini scripting language? Or will there be a more abstract way of providing the same intention to the tool? If editor integrations are expected to send the script-snippets, will there be facilities for programmatically generating valid script snippets?

alert0 5y ago

I've always wanted a tool that finds duplicate code (with minor variable name differences) so I can factor it out. Often when prototyping I'll copy a function thinking it will change a lot or maybe just not knowing what will change. As the application matures I'll go back and clean these up but it shouldn't be too hard for a tool to identify.

thegeekpirate 5y ago
https://github.com/mibk/dupl https://github.com/mibk/dupl does a great job, and it's included in https://github.com/golangci/golangci-lint https://github.com/golangci/golangci-lint (which everyone should use, tbh)
AlphaSite 5y ago
IntelliJ does this.
grok22 5y ago
https://pmd.github.io/latest/pmd_userdocs_cpd.html https://pmd.github.io/latest/pmd_userdocs_cpd.html

konart 5y ago

Shouldn't this be part of gopls or something?

onionisafruit 5y ago
Maybe eventually, but not while it's experimental.

iddan 5y ago

A similar concept named "Tactics" was explored in Haskell. https://reasonablypolymorphic.com/blog/towards-tactics/ https://reasonablypolymorphic.com/blog/towards-tactics/ HN Discussion: https://news.ycombinator.com/item?id=24759649 https://news.ycombinator.com/item?id=24759649

shirogane86x 5y ago
Relevant: https://haskellwingman.dev/ https://haskellwingman.dev/ . it's actually possible to use it if you have the haskell language server installed, as a code action (at least i think it is, haven't personally tried it yet tho)

dorian-graph 5y ago

A similar, but general purpose, tool to this is https://comby.dev/ https://comby.dev/. I've used it before locally for some structural code changes. The author is super helpful/responsive too.

MorganGallant 5y ago

I wonder if this is some sort of precursor work for the generics release? Feels like gofmt -r on steroids, and could be eventually used to do large scale transformations to the standard library?