-
Notifications
You must be signed in to change notification settings - Fork 1
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
Initial version. #1
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
#! /bin/sh | ||
|
||
set -e | ||
|
||
export CARGO_HOME="`pwd`/.cargo" | ||
export RUSTUP_HOME="`pwd`/.rustup" | ||
|
||
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > rustup.sh | ||
sh rustup.sh --default-host x86_64-unknown-linux-gnu --default-toolchain stable -y --no-modify-path | ||
|
||
export PATH=`pwd`/.cargo/bin/:$PATH | ||
|
||
cargo fmt --all -- --check | ||
cargo test | ||
|
||
which cargo-deny | cargo install cargo-deny || true | ||
if [ "X`which cargo-deny`" != "X"]; then | ||
cargo-deny check license | ||
else | ||
echo "Warning: couldn't run cargo-deny" > /dev/stderr | ||
fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
/target | ||
Cargo.lock |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
[package] | ||
name = "depub" | ||
version = "0.1.0" | ||
authors = ["Laurence Tratt <laurie@tratt.net>"] | ||
edition = "2018" | ||
readme = "README.md" | ||
license = "Apache-2.0 OR MIT" | ||
|
||
[dependencies] | ||
getopts = "0.2" | ||
regex = "1.4" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
Licensed under the Apache License, Version 2.0 (the "License"); you may not use | ||
this file except in compliance with the License. You may obtain a copy of the | ||
License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software distributed | ||
under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR | ||
CONDITIONS OF ANY KIND, either express or implied. See the License for the | ||
specific language governing permissions and limitations under the License. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
Permission is hereby granted, free of charge, to any person obtaining a copy of | ||
this software and associated documentation files (the "Software"), to deal in | ||
the Software without restriction, including without limitation the rights to | ||
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies | ||
of the Software, and to permit persons to whom the Software is furnished to do | ||
so, subject to the following conditions: | ||
|
||
The above copyright notice and this permission notice shall be included in all | ||
copies or substantial portions of the Software. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
SOFTWARE. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,79 @@ | ||
# depub | ||
# depub: minimise visibility | ||
|
||
## Overview | ||
|
||
When working on medium or large sized Rust code bases, it can be hard to know | ||
whether the visibility of functions, structs, and so on are still at the | ||
minimum required. For example, sometimes functions that once needed to be `pub` | ||
now only need to be `pub(crate)`, `pub(super)`, or simply private. | ||
|
||
`depub` minimises the visibility of such items in files passed to it, using a | ||
user-specified command (e.g. `cargo check`) as an oracle to tell if its | ||
reduction of an item's visibility is valid or not. Note that `depub` is | ||
entirely guided by the oracle command: if the code it compiles happens not to | ||
use part of an intentionally public interface, then `depub` is likely to | ||
suggest reducing its visibility even though that's not what you want. The | ||
broader the coverage of your oracle, the less this is an issue. | ||
|
||
In essence, `depub` does a string search for `pub`, replaces it with `pub | ||
crate` and sees if a test command still succeeds. If it does, it keeps that | ||
visibility, otherwise it replaces with the original and tries the next item. | ||
Note that `depub` is inherently destructive: it overwrites files as it | ||
operates, so do not run it on source code that you do not want altered! | ||
|
||
The list of visibilities that `depub` considers is, in order: `pub`, | ||
`pub(crate)`, `pub(super)`, and private (i.e. no `pub` keyword at all). `depub` | ||
searches for `pub`/`pub(crate)`/`pub(super)` instances, reduces their | ||
visibility by one level, and tries the oracle command. If it succeeds, it tries | ||
the next lower level until private visibility has been reached. | ||
|
||
Since reducing the visibility of one item can enable other items' visibility to | ||
be reduced, `depub` keeps running "rounds" until a fixed point has been | ||
reached. The maximum number of rounds is equal to the number of visible items | ||
in the code base, though in practise 2 or 3 rounds are likely to be all that is | ||
needed. | ||
|
||
|
||
## Usage | ||
|
||
`depub`'s usage is as follows: | ||
|
||
``` | ||
depub -c <command> file_1 [... file_n] | ||
``` | ||
|
||
where `<command>` is a string to be passed to `/bin/sh -c` for execution to | ||
determine whether the altered source code is still valid. | ||
|
||
To reduce the visibility of a normal Rust project, `cd` to your Rust code base | ||
and execute: | ||
|
||
``` | ||
$ find . -name "*.rs" | \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is begging for a cargo plugin to hide the file search from users. Just as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Way too much complexity for now! Also, at least at the moment, it's sometimes useful to only pass a subset of files. |
||
xargs /path/to/depub -c "cargo check && cargo check --test" | ||
``` | ||
|
||
`depub` informs you of its progress. After it is finished, `diff` your code | ||
base, and accept those of its recommendations you think appropriate. Note that | ||
`depub` currently uses string search and replace, so it will merrily change the | ||
string `pub` in a command into `pub(crate)` -- you should not expect to accept | ||
its recommendations without at least a cursory check. | ||
|
||
|
||
## Using with libraries | ||
|
||
Running `depub` on a library will tend to reduce all its intentionally `pub` | ||
functions to private visibility. You can weed these out manually after `depub` | ||
has run, but this can be tedious, and may also have reduced the visibility of a | ||
cascade of other items. | ||
|
||
To avoid this, use one or more users of the library in the oracle command as part | ||
of your oracle. Temporarily alter their `Cargo.toml` to point to the local | ||
version of your libary and use a command such as: | ||
|
||
``` | ||
$ find . -name "*.rs" | \ | ||
xargs /path/to/depub -c " \ | ||
cargo check && cargo check --test && \ | ||
cd /path/to/lib && cargo check && cargo check --test" | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
status = ["buildbot/buildbot-build-script"] | ||
|
||
timeout_sec = 300 # 10 minutes | ||
|
||
# Have bors delete auto-merged branches | ||
delete_merged_branches = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
use getopts::Options; | ||
use regex::RegexBuilder; | ||
use std::{ | ||
env, | ||
fs::{read_to_string, write}, | ||
io::{stdout, Write}, | ||
path::Path, | ||
process::{self, Command, Stdio}, | ||
}; | ||
|
||
enum PubKind { | ||
Pub, | ||
Crate, | ||
Super, | ||
Private, | ||
} | ||
|
||
fn process(oracle_cmd: &str, p: &Path) -> u64 { | ||
let pub_regex = RegexBuilder::new("pub(?:\\s*\\(\\s*(.*?)\\s*\\))?") | ||
.multi_line(true) | ||
.build() | ||
.unwrap(); | ||
let mut cur_txt = read_to_string(p).unwrap(); | ||
let mut i = 0; | ||
let mut cl = pub_regex.capture_locations(); | ||
let mut num_changed = 0; | ||
while i < cur_txt.len() { | ||
print!("."); | ||
stdout().flush().ok(); | ||
let old_txt = cur_txt.clone(); | ||
let m = match pub_regex.captures_read_at(&mut cl, &old_txt, i) { | ||
Some(m) => m, | ||
None => break, | ||
}; | ||
let mut kind = if let Some((start, end)) = cl.get(1) { | ||
match &cur_txt[start..end] { | ||
"crate" => PubKind::Crate, | ||
"super" => PubKind::Super, | ||
_ => { | ||
// FIXME: this captures things we don't need to deal with (e.g. `pub(self)`), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A future version of this tool could use a parser (or maybe rust-analyzer) so as to only mutate things that really are visibility modifiers? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably one should plug into rustc. That's no small job! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's why rust-analyzer is probably more attractive. But anyway, we won't do that right now. |
||
// things we could deal with (e.g. `pub(in ...)`) and random strings we've | ||
// accidentally picked up (e.g. `a pub(The Frog and Cow)`). We should probably | ||
// do something better with the middle class of thing than simply ignoring it. | ||
i = m.end(); | ||
continue; | ||
} | ||
} | ||
} else { | ||
PubKind::Pub | ||
}; | ||
let mut next_txt = cur_txt.clone(); | ||
let mut depubed = false; | ||
loop { | ||
let next_kind = match kind { | ||
PubKind::Pub => PubKind::Crate, | ||
PubKind::Crate => PubKind::Super, | ||
PubKind::Super => PubKind::Private, | ||
PubKind::Private => break, | ||
}; | ||
let mut try_txt = cur_txt[..m.start()].to_string(); | ||
let pub_txt = match next_kind { | ||
PubKind::Crate => "pub(crate) ", | ||
PubKind::Super => "pub(super) ", | ||
PubKind::Private => "", | ||
_ => unreachable!(), | ||
}; | ||
try_txt.push_str(&pub_txt); | ||
try_txt.push_str(&cur_txt[m.end()..]); | ||
write(p, &try_txt).unwrap(); | ||
match Command::new("sh") | ||
.arg("-c") | ||
.arg(oracle_cmd) | ||
.stderr(Stdio::null()) | ||
.stdout(Stdio::null()) | ||
.status() | ||
{ | ||
Ok(s) if s.success() => { | ||
if !depubed { | ||
num_changed += 1; | ||
depubed = true; | ||
} | ||
next_txt = try_txt; | ||
} | ||
_ => break, | ||
} | ||
kind = next_kind; | ||
} | ||
cur_txt = next_txt; | ||
if cur_txt.len() >= old_txt.len() { | ||
i = m.end() + (cur_txt.len() - old_txt.len()); | ||
} else { | ||
i = m.end() - (old_txt.len() - cur_txt.len()); | ||
} | ||
} | ||
write(p, cur_txt).unwrap(); | ||
num_changed | ||
} | ||
|
||
fn progname() -> String { | ||
match env::current_exe() { | ||
Ok(p) => p | ||
.file_name() | ||
.map(|x| x.to_str().unwrap_or("depub")) | ||
.unwrap_or("depub") | ||
.to_owned(), | ||
Err(_) => "depub".to_owned(), | ||
} | ||
} | ||
|
||
/// Print out program usage then exit. This function must not be called after daemonisation. | ||
fn usage() -> ! { | ||
eprintln!( | ||
"Usage: {} -c <command> file_1 [... file_n]", | ||
progname = progname() | ||
); | ||
process::exit(1) | ||
} | ||
|
||
fn main() { | ||
let matches = Options::new() | ||
.reqopt("c", "command", "Command to execute.", "string") | ||
.optflag("h", "help", "") | ||
.parse(env::args().skip(1)) | ||
.unwrap_or_else(|_| usage()); | ||
if matches.opt_present("h") || matches.free.is_empty() { | ||
usage(); | ||
} | ||
|
||
let oracle_cmd = matches.opt_str("c").unwrap(); | ||
let mut round = 1; | ||
loop { | ||
println!("===> Round {}", round); | ||
let mut changed = false; | ||
for p in &matches.free { | ||
print!("{}: ", p); | ||
stdout().flush().ok(); | ||
let num_changed = process(oracle_cmd.as_str(), &Path::new(&p)); | ||
if num_changed > 0 { | ||
print!(" ({} items depub'ed)", num_changed); | ||
changed = true; | ||
} | ||
println!(""); | ||
} | ||
if !changed { | ||
break; | ||
} | ||
round += 1; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need some kind of warning stating that: in general you cannot know what
pub
stuff downstream crates may consume, so you should only really use this tool in "enclosed ecosystems".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that was sort-of obvious in this bit:
but maybe it isn't?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I knew what you meant, but since the implications of getting it wrong could be quite bad, I don't think it hurts to make it explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed (hopefully) in fc38494.