Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write autofix functions for existing rules #94

Open
8 of 13 tasks
duaraghav8 opened this issue Aug 4, 2017 · 40 comments
Open
8 of 13 tasks

Write autofix functions for existing rules #94

duaraghav8 opened this issue Aug 4, 2017 · 40 comments

Comments

@duaraghav8
Copy link
Owner

duaraghav8 commented Aug 4, 2017

  • indentation - is a top priority (is also very complicated and indentation rules needs to be re-written in simpler way before we implement its autofix)
  • whitespace
  • lbrace
  • pragma-on-top's inspectExperimentalPragmaStatement()
  • imports-on-top
  • blank-lines
  • operator-whitespace
  • function-ordering
  • array-declarations
  • deprecated-suicide
  • pragma-on-top
  • quotes
  • emit

See dev guide on how to implement fix()es for rules.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 150.0 DAI (150.0 USD @ $1.0/DAI) attached to it.

@duaraghav8
Copy link
Owner Author

cc @nemaniarjun (if you're interested)

@nemani
Copy link
Contributor

nemani commented Jul 5, 2018

I would be interested, but I am busy this week, maybe able to pick this up after the 15th. Also, I would suggest breaking the bounty into smaller parts / or making it a "Co-operative" bounty, which will result in faster work IMO.

@gomesalexandre
Copy link

If this is still opened, I could have a look at it this week-end.
I really like what you guys do for Solidity devs experience so that would be a good first contribution to get familiar with the project.

@gitcoinbot
Copy link

gitcoinbot commented Jul 7, 2018

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 2 years, 5 months ago.
Please review their action plans below:

1) ihordiachenko has started work.

I'm going to start with the operator-whitespace rule to get familiar with the ethlint internal APIs. If the PR is fine for you, I'll assess the remaining rules.

Learn more on the Gitcoin Issue Details page.

@duaraghav8
Copy link
Owner Author

Thanks @gomesalexandre :)
Let me know if you face any problems

@gomesalexandre
Copy link

gomesalexandre commented Jul 8, 2018

  • operator-whitespace is working, needs tests
  • As far as function-ordering goes I've spent a few hours on it without success yet.
    It is not working, as in not fixing everything in one go, you have to run the command a few times before everything becomes in correct order.
    My question is, is there a way to directly interact with the array of function node objects and modify their order, then "reinject it" or do you absolutely have to use the fixer helper ? If it's the former, I got the logic pretty much done already, if the latter I'll need more time !

@duaraghav8
Copy link
Owner Author

directly interact with the array of function node objects and modify their order - No. The number of edge cases to deal with when directly modifying the AST outweigh the benefits, so I never went ahead with it.
I understand the problem you're facing with func order. There seems to be no easy solution to this. The best I can think of will be a nightmare to maintain down the line (I'll be happy to describe it to you). So I guess, we'll just have to abandon the fix function for this rule.

@gomesalexandre
Copy link

I see your point, makes sense. It's always good to have sane abstractions like the fixer one. That's also the approach that eslint took and is a way safer and maintainable one.
I'll be happy to hear about that solution, in the meantime I'll check the other rules !

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 150.0 DAI (150.0 USD @ $1.0/DAI) attached to it.

@gomesalexandre
Copy link

@aerophile I am not sure why this was reopened by the Gitcoin bot, however, it's a collaborative bounty so feel free to take on the rules I haven't worked on 👍
I started work on function-order and operator-whitespace already (see PR here) so you could start working on the other rules, as I plan to finish these by next week.

@nemani nemani mentioned this issue Jul 14, 2018
4 tasks
@aerophile
Copy link

aerophile commented Jul 15, 2018

sure @gomesalexandre, considering that the remaining three items are common to this and #169 It will be better to get inputs from @duaraghav8 before proceeding to avoid redundant work.

@gomesalexandre
Copy link

gomesalexandre commented Jul 17, 2018

@duaraghav8 @aerophile Pushed some more fixes for operator-whitespace, I miss a few more autofixes on this one that I plan on fixing tomorrow, and also finish function-order if I have time.
From there, it also means that improving operator-whitespace (as part of #169) can be done by whoever wants to !

@aerophile
Copy link

aerophile commented Jul 21, 2018

@gomesalexandre @duaraghav8 Just an update from my side: Have been reading documentation and really got the gist of what solium is and how rules and fixing work. I really appreciate the architectural choices made in v1.0+ . I think I'll be working on the autofix functions for lbrace and whitespace.

@mkosowsk
Copy link

Hi @gomesalexandre and @duaraghav8, just a friendly check-in here 😀 how are things coming along?

@gomesalexandre
Copy link

Hey @mkosowsk unfortunately I didn't have time to continue the work on it yet. I'll try to push the two rules' missing autofixes on Sunday night Paris time (in 48 hours essentially), but can't confirm 😞. If I didn't push it by then, feel free to write the remaining fixes, including the ones for these two(you can fork my fork and use that as a basis !)

@mkosowsk
Copy link

mkosowsk commented Aug 1, 2018

Hi @aerophile per @gomesalexandre last comment, do you think you have the tools to run with this issue? Thanks!

@spm32
Copy link

spm32 commented Aug 13, 2018

Hey @aerophile @gomesalexandre just wanted to follow up on this once more, any updates?

@aerophile
Copy link

@ceresstation Expecting to update and complete by this weekend

@spm32
Copy link

spm32 commented Aug 21, 2018

Hey @aerophile did you manage to find time to finish this up over the weekend as mentioned?

@aerophile
Copy link

@ceresstation I did spend time over the weekend but I'm still on it. I plan to continue.

@spm32
Copy link

spm32 commented Sep 18, 2018

@aerophile Hey there, we haven't heard back from you in a while, are you still working on this?

@frankchen07
Copy link

@aerophile - pinging again to see if you're still working on this!

@codi45
Copy link

codi45 commented Oct 25, 2018

hi i would like to konw if this bounty is still open?
if yes which rules remains to be implemented?
thanks

@frankchen07
Copy link

hey @duaraghav8, Frank from Gitcoin here, is this issue still open for @codi45?

@duaraghav8
Copy link
Owner Author

@frankchen07 hey even I haven't heard from aerophile so if this issue is not already open but @codi45 is keen on working on it, could you help re-open it on gitcoin? Because I now consider this issue to be open again to hunters

@codi45
Copy link

codi45 commented Oct 29, 2018

Hi @duaraghav8 yes i would like to start and will want to know the issues because i try the lbrace rule and it is working correctly if the brackets are there or not so i would to know more about the fix.

@frankchen07
Copy link

@codi45 & @duaraghav8 - yep the issue is still open! https://gitcoin.co/issue/duaraghav8/Solium/94/722

@spm32
Copy link

spm32 commented Nov 6, 2018

Hey @codi45 are you still interested in trying out this issue?

@dev1644
Copy link

dev1644 commented Jan 3, 2019

Hey, @duaraghav8 @ceresstation is this issue still open?

@duaraghav8
Copy link
Owner Author

@dev1644 going by the issue details page, yes it is open. I'd still wait for @ceresstation's confirmation

@dev1644
Copy link

dev1644 commented Jan 3, 2019

@duaraghav8 Ok, once @ceresstation confirm it, I will start working on it.

@spm32
Copy link

spm32 commented Jan 23, 2019

Hey @dev1644 really sorry for the delay, you're good to go here!

@ihordiachenko
Copy link

Hey @duaraghav8. Is the bounty still attached to this issue?

@spm32
Copy link

spm32 commented Aug 1, 2019

Hi @ihordiachenko it should be, if you can get the remaining tasks complete that would be super helpful.

@duaraghav8
Copy link
Owner Author

duaraghav8 commented Aug 2, 2019

@ihordiachenko Let me know in case you have any doubts

@ihordiachenko
Copy link

@ceresstation @duaraghav8 cool, I'll give it a shot next week

@ihordiachenko
Copy link

@duaraghav8 autofix for the operator-whitespace rule is ready. Could you take a look at it?

#270

@duaraghav8
Copy link
Owner Author

@ihordiachenko sorry for the late response, I had been on leave for a while.
Thanks for the PR, will review it in a day's time.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 1 year, 8 months ago.
Please review their action plans below:

1) akanimorex has started work.

i am very proficient in javascript and i can work on this if this is still open

Learn more on the Gitcoin Issue Details page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests