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

Add ?clippy command #74

Open
technetos opened this issue Dec 26, 2020 · 6 comments
Open

Add ?clippy command #74

technetos opened this issue Dec 26, 2020 · 6 comments
Labels
good first issue Good for newcomers

Comments

@technetos
Copy link
Member

play.rust-lang.org supports running code through clippy. Add this support to the bot so users can run code from discord through clippy.

@technetos technetos changed the title Add a ?clippy command Add ?clippy command Dec 26, 2020
@technetos technetos added the low hanging fruit easy stuff that isnt a new feature label Jan 26, 2021
@kangalio
Copy link

Are the other playground feature (Rusfmt, Miri, Expand macros) desirable as separate commands as well?

@technetos
Copy link
Member Author

So, expand macros would be awesome. The rest im not convinced are worth doing.

@technetos technetos added good first issue Good for newcomers and removed low hanging fruit easy stuff that isnt a new feature labels Jan 29, 2021
@kangalio
Copy link

kangalio commented Feb 1, 2021

A question
For regular code execution we have two variants to dictate how the code should be sent to the playground: ?eval and ?play. How should this be handled with the new ?clippy/?expand command?

Some ideas

  1. ?clippyeval and ?expandeval variant
  2. Automatically detect eval vs play mode by checking for the presence of "fn main" in the code
  3. Don't support eval-like execution at all

1 is a bit clunky but functioning, 2 is a bit magic but pleasant to use, 3 is unfortunate because for ?clippy and for example for testing custom macros with ?expand, eval-like functionality would be very convenient. I think I personally prefer 2

Another question
Should the bot automatically run rustfmt on the macro-expanded code? Macro-expanded code is notoriously ugly; see this expanded hello world program:

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::v1::*;
#[macro_use]
extern crate std;
fn main() {
    {
        ::std::io::_print(::core::fmt::Arguments::new_v1(&["Hello world\n"],
                                                         &match () {
                                                              () => [],
                                                          }));
    };
}

To avoid the second network request wait (playground is kinda sluggish), rustfmt could be executed locally I think. The computation expense is negligible, though I haven't checked if rustfmt has code execution vulnerabilities or anything like that-

@khionu
Copy link
Member

khionu commented Feb 1, 2021

?clippy should run its own request and have nothing to do with ?play or ?eval. It's a totally separate operation.

Re formatting expansions, I would say sure, but we would need to look to make sure local code execution isn't a problem.

@kangalio
Copy link

kangalio commented Feb 1, 2021

?clippy should run its own request and have nothing to do with ?play or ?eval. It's a totally separate operation.

I understand that. I was merely making a comparison to ?play/?eval, asking if the clippy and expand commands should have a similar distinction of evaluation vs plain executing.

Re formatting expansions, I would say sure, but we would need to look to make sure local code execution isn't a problem.

Yeah, as said above, I have the same concerns.

@khionu
Copy link
Member

khionu commented Feb 1, 2021

... if the clippy and expand commands should have a similar distinction of evaluation vs plain executing.

The distinction between play and eval could be removed, but it would require preprocessing that was not worth investing in for the initial iteration of the functionality. The distinction exists strictly for the sake of output handling. There is no variation in how to handle output of clippy or expand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants