[WIP] Add localtofield refactoring based on rope functionality

[tonybaloney]

I would like to add more of the functionality of rope into this plugin. I've started with the simplest and a very useful local-to-field refactoring command in the hope of getting a little bit of guidance and then tackling more complicated refactorings (those which need user input)

I don't think this PR includes all of the required TS changes, but I couldn't find one of the existing refactor methods to use as a template so I've marked this as WIP.

What does simpleRefactorProvider.ts do?

• Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
• Title summarizes what is changing
• Has a news entry file (remember to thank yourself!)
• Unit tests & system/integration tests are added/updated
• Test plan is updated as appropriate
• package-lock.json has been regenerated by running npm install (if dependencies have changed)

[d3r3kk]

Hi @tonybaloney, thanks for the contribution! I'll try to help with answering questions.

First, we generally work off of issues in our repo. Is there an issue that corresponds to the work that you are proposing? If there isn't one please feel free to create one and link it to this PR.

As for your questions:

I don't think this PR includes all of the required TS changes...

We would generally take contributions after there are adequate TS unit / system tests in place that fully prove out the feature (as best as possible of course) within our CI.

...I couldn't find one of the existing refactor methods to use as a template so I've marked this as WIP.
What does simpleRefactorProvider.ts do?

You've discovered the existing refactor methods! Within simpleRefactorProvider you will see a single exported function activateSimplePythonRefactorProvider. Within that function you will see two pertinent registrations on line 18 and 29 - these register the commands that invoke refactoring. Note that they are functors that are registered into the system (see the () => { ... } formatting?).

HTH.

A bit of a heads up, our team will review new PRs from external contributors generally in the next following sprint - so we might not get to reviews until January.

[tonybaloney]

@d3r3kk thanks for all the pointers. I’ll raise an issue as well.

[tonybaloney]

@d3r3kk done the end-to-end TS pieces, now just need to write the tests.

[codecov]

Codecov Report

Merging #3825 into master will decrease coverage by 1%.
The diff coverage is 78%.

@@           Coverage Diff           @@
##           master   #3825    +/-   ##
=======================================
- Coverage      79%     79%   -<1%     
=======================================
  Files         387     399    +12     
  Lines       17834   18278   +444     
  Branches     2879    2949    +70     
=======================================
+ Hits        13974   14286   +312     
- Misses       3856    3989   +133     
+ Partials        4       3     -1

Flag	Coverage Δ
#Linux	68% <78%> (ø)	⬇️
#Windows	53% <41%> (-15%)	⬇️
#macOS	67% <78%> (ø)	⬇️

Impacted Files	Coverage Δ
src/client/common/constants.ts	100% <100%> (ø)	⬆️
src/client/telemetry/constants.ts	100% <100%> (ø)	⬆️
src/client/refactor/proxy.ts	81% <72%> (-3%)	⬇️
src/client/providers/simpleRefactorProvider.ts	81% <78%> (ø)	⬇️
...rc/client/common/errors/moduleNotInstalledError.ts	34% <0%> (-66%)	⬇️
src/client/linters/errorHandlers/notInstalled.ts	45% <0%> (-55%)	⬇️
src/client/linters/errorHandlers/standard.ts	40% <0%> (-55%)	⬇️
src/client/linters/prospector.ts	53% <0%> (-34%)	⬇️
src/client/common/utils/text.ts	74% <0%> (-26%)	⬇️
...c/client/linters/errorHandlers/baseErrorHandler.ts	78% <0%> (-22%)	⬇️
... and 69 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update facaaed...91f85de. Read the comment docs.

[tonybaloney]

@d3r3kk Probably a silly question. But how do I get the tests to actually run?

The scripts seem to have unittests and functional tests. The functional tests download the extension from a website (which is not what I want)

The unittests seem to target the build version, not the source version?

"test": "node ./out/test/standardTest.js && node ./out/test/multiRootTest.js",
        "test:unittests": "mocha --require source-map-support/register --opts ./build/.mocha.unittests.opts",
        "test:unittests:cover": "nyc --nycrc-path ./build/.nycrc npm run test:unittests",
        "test:functional": "mocha --require source-map-support/register --opts ./build/.mocha.functional.opts",
        "test:functional:cover": "nyc --nycrc-path ./build/.nycrc npm run test:functional",
        "testDebugger": "node ./out/test/debuggerTest.js",
        "testSingleWorkspace": "node ./out/test/standardTest.js",
        "testMultiWorkspace": "node ./out/test/multiRootTest.js",
        "testPerformance": "node ./out/test/performanceTest.js",
        "testSmoke": "node ./out/test/smokeTest.js",

[d3r3kk]

To run them locally, you can follow the guide in our CONTRIBUTING.md document here.

[DonJayamanne]

Closing this PR as this has been stale for over a month