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

Lint vertical spacing in Rust code #1572

Merged
merged 6 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/re_data_store/src/entity_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub struct EntityProperties {
///
/// See [`Self::color_mapper`] to select an actual mapping.
pub color_mapping: bool,

/// What kind of color mapping should be applied (none, map, texture, transfer..)?
pub color_mapper: EditableAutoValue<ColorMapper>,

Expand All @@ -64,12 +65,15 @@ pub struct EntityProperties {
/// Only applies to tensors with meaning=depth that are affected by a pinhole transform when
/// in a spatial view, using 3D navigation.
pub backproject_depth: bool,

/// Entity path of the pinhole transform used for the backprojection.
///
/// `None` means backprojection is disabled.
pub backproject_pinhole_ent_path: Option<EntityPath>,

/// Used to scale the resulting point cloud.
pub backproject_scale: EditableAutoValue<f32>,

/// Used to scale the radii of the points in the resulting point cloud.
pub backproject_radius_scale: EditableAutoValue<f32>,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ impl BinaryBuffer {

impl Index<usize> for BinaryBuffer {
type Output = u8;

fn index(&self, i: usize) -> &u8 {
&self.0[i]
}
Expand Down
2 changes: 2 additions & 0 deletions crates/re_renderer/examples/depth_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ struct DepthTexture {
dimensions: glam::UVec2,
data: DepthCloudDepthData,
}

impl DepthTexture {
pub fn spiral(dimensions: glam::UVec2) -> Self {
let size = (dimensions.x * dimensions.y) as usize;
Expand All @@ -429,6 +430,7 @@ struct AlbedoTexture {
dimensions: glam::UVec2,
rgba8: Vec<u8>,
}

impl AlbedoTexture {
pub fn spiral(dimensions: glam::UVec2) -> Self {
let size = (dimensions.x * dimensions.y) as usize;
Expand Down
2 changes: 2 additions & 0 deletions crates/re_renderer/src/allocator/uniform_buffer_fill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{wgpu_resources::BindGroupEntry, DebugLabel, RenderContext};
struct UniformBufferAlignmentCheck<T> {
pub _marker: std::marker::PhantomData<T>,
}

impl<T> UniformBufferAlignmentCheck<T> {
/// wgpu requires uniform buffers to be aligned to up to 256 bytes.
///
Expand Down Expand Up @@ -33,6 +34,7 @@ impl<T> UniformBufferAlignmentCheck<T> {
"Uniform buffers need to be aligned to 256 bytes. Use `#[repr(C, align(256))]`"
);
}

/// Utility for fast & efficient creation of uniform buffers from a series of structs.
///
/// For subsequent frames, this will automatically not allocate any resources (thanks to our buffer pooling mechanism).
Expand Down
2 changes: 2 additions & 0 deletions crates/re_renderer/src/renderer/mesh_renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ mod gpu_data {
#[derive(Clone)]
struct MeshBatch {
mesh: GpuMesh,

count: u32,

/// Number of meshes out of `count` which have outlines.
/// We put all instances with outlines at the start of the instance buffer range.
count_with_outlines: u32,
Expand Down
2 changes: 2 additions & 0 deletions crates/re_renderer/src/renderer/outlines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ pub struct OutlineConfig {

/// Premultiplied RGBA color for the first outline layer.
pub color_layer_a: crate::Rgba,

/// Premultiplied RGBA color for the second outline layer.
pub color_layer_b: crate::Rgba,
}
Expand Down Expand Up @@ -143,6 +144,7 @@ mod gpu_data {
#[derive(Clone, Copy, bytemuck::Pod, bytemuck::Zeroable)]
pub struct JumpfloodingStepUniformBuffer {
pub step_width: wgpu_buffer_types::U32RowPadded,

/// All this padding hurts. `step_width` be a PushConstant but they are not widely supported enough!
pub end_padding: [wgpu_buffer_types::PaddingRow; 16 - 1],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ where
.load(std::sync::atomic::Ordering::Relaxed)
}
}

impl<Handle, Desc, Res> Drop for DynamicResourcePool<Handle, Desc, Res>
where
Handle: Key,
Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/wgpu_resources/texture_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ impl DynamicResourcesDesc for TextureDesc {
true
}
}

#[derive(Default)]
pub struct GpuTexturePool {
pool: DynamicResourcePool<GpuTextureHandle, TextureDesc, GpuTextureInternal>,
Expand Down
2 changes: 2 additions & 0 deletions crates/re_viewer/src/remote_viewer_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ pub struct RemoteViewerApp {
app_env: crate::AppEnvironment,
startup_options: crate::StartupOptions,
re_ui: re_ui::ReUi,

/// The url of the remote server.
url: String,

app: Option<(re_ws_comms::Connection, App)>,
}

Expand Down
1 change: 1 addition & 0 deletions crates/re_viewer/src/ui/view_spatial/ui_renderer_bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ fn create_and_fill_view_builder(
}

slotmap::new_key_type! { pub struct ViewBuilderHandle; }

type ViewBuilderMap = slotmap::SlotMap<ViewBuilderHandle, ViewBuilder>;

fn renderer_paint_callback(
Expand Down
4 changes: 4 additions & 0 deletions examples/rust/api_demo/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ fn demo_extension_components(session: &Session) -> anyhow::Result<()> {
#[derive(arrow2_convert::ArrowField, arrow2_convert::ArrowSerialize)]
#[arrow_field(transparent)]
struct Confidence(f32);

impl Component for Confidence {
fn name() -> ComponentName {
"ext.confidence".into()
Expand All @@ -102,14 +103,17 @@ fn demo_extension_components(session: &Session) -> anyhow::Result<()> {
#[derive(arrow2_convert::ArrowField, arrow2_convert::ArrowSerialize)]
#[arrow_field(transparent)]
struct Corner(String);

impl Component for Corner {
fn name() -> ComponentName {
"ext.corner".into()
}
}

#[derive(arrow2_convert::ArrowField, arrow2_convert::ArrowSerialize)]
#[arrow_field(transparent)]
struct Training(bool);

impl Component for Training {
fn name() -> ComponentName {
"ext.training".into()
Expand Down
1 change: 1 addition & 0 deletions examples/rust/objectron/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ struct ArFrame {
index: usize,
timepoint: TimePoint,
}

impl ArFrame {
fn from_raw(
dir: PathBuf,
Expand Down
165 changes: 157 additions & 8 deletions scripts/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
import os
import re
import sys
from typing import Optional
from typing import Any, List, Optional, Tuple

# -----------------------------------------------------------------------------

todo_pattern = re.compile(r"TODO([^(]|$)")
debug_format_of_err = re.compile(r"\{\:#?\?\}.*, err")
Expand Down Expand Up @@ -65,7 +67,7 @@ def lint_line(line: str) -> Optional[str]:
return None


def test_lint() -> None:
def test_lint_line() -> None:
assert lint_line("hello world") is None

should_pass = [
Expand Down Expand Up @@ -109,7 +111,137 @@ def test_lint() -> None:
assert lint_line(line) is not None, f'expected "{line}" to fail'


def lint_file(filepath: str) -> int:
# -----------------------------------------------------------------------------

re_declaration = re.compile(r"^\s*((pub(\(\w*\))? )?((impl|fn|struct|enum|union|trait|type)\b))")
re_attribute = re.compile(r"^\s*\#\[(error|derive)")
re_docstring = re.compile(r"^\s*///")


def is_missing_blank_line_between(prev_line: str, line: str) -> bool:
def is_empty(line: str) -> bool:
return (
line == ""
or line.startswith("#")
or line.startswith("//")
or line.endswith("{")
or line.endswith("(")
or line.endswith("\\")
or line.endswith('r"')
or line.endswith("]")
)

"""Only for Rust files."""
if re_declaration.match(line) or re_attribute.match(line) or re_docstring.match(line):
line = line.strip()
prev_line = prev_line.strip()

if is_empty(prev_line):
return False

if line.startswith("fn ") and line.endswith(";"):
return False # maybe a trait function

if line.startswith("type ") and prev_line.endswith(";"):
return False # many type declarations in a row is fine

if prev_line.endswith(",") and line.startswith("impl"):
return False

if prev_line.endswith("*"):
return False # maybe in a macro

return True

return False


def lint_vertical_spacing(lines_in: List[str]) -> Tuple[List[str], List[str]]:
"""Only for Rust files."""
prev_line = None

errors = []
lines_out = []

for line_nr, line in enumerate(lines_in):
line_nr = line_nr + 1

if prev_line is not None and is_missing_blank_line_between(prev_line, line):
errors.append(f"{line_nr}: for readability, add newline before `{line.strip()}`")
lines_out.append("\n")

lines_out.append(line)
prev_line = line

return errors, lines_out


def test_lint_vertical_spacing() -> None:
should_pass = [
"hello world",
"""
/// docstring
foo

/// docstring
bar
""",
"""
trait Foo {
fn bar();
fn baz();
}
""",
# macros:
"""
$(#[$meta])*
#[derive(Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
""",
"""
Item = (
&PointCloudBatchInfo,
impl Iterator<Item = &PointCloudVertex>,
),
""",
"""
type Response = Response<Body>;
type Error = hyper::Error;
""",
]

should_fail = [
"""
/// docstring
foo
/// docstring
bar
""",
"""
Foo,
#[error]
Bar,
""",
"""
slotmap::new_key_type! { pub struct ViewBuilderHandle; }
type ViewBuilderMap = slotmap::SlotMap<ViewBuilderHandle, ViewBuilder>;
""",
]

for code in should_pass:
errors, _ = lint_vertical_spacing(code.split("\n"))
assert len(errors) == 0, f"expected this to pass:\n{code}\ngot: {errors}"

for code in should_fail:
errors, _ = lint_vertical_spacing(code.split("\n"))
assert len(errors) > 0, f"expected this to fail:\n{code}"

pass


# -----------------------------------------------------------------------------


def lint_file(filepath: str, args: Any) -> int:
with open(filepath) as f:
lines_in = f.readlines()

Expand All @@ -121,11 +253,26 @@ def lint_file(filepath: str) -> int:
num_errors += 1
print(f"{filepath}:{line_nr+1}: {error}")

if filepath.endswith(".rs"):
errors, lines_out = lint_vertical_spacing(lines_in)

for error in errors:
print(f"{filepath}{error}")

if args.fix and lines_in != lines_out:
with open(filepath, "w") as f:
f.writelines(lines_out)
print(f"{filepath} fixed.")

num_errors = len(errors)

return num_errors


def main() -> None:
test_lint() # Make sure we are bug free before we run!
# Make sure we are bug free before we run:
test_lint_line()
test_lint_vertical_spacing()

parser = argparse.ArgumentParser(description="Lint code with custom linter.")
parser.add_argument(
Expand All @@ -135,14 +282,15 @@ def main() -> None:
nargs="*",
help="File paths. Empty = all files, recursively.",
)
parser.add_argument("--fix", dest="fix", action="store_true", help="Automatically fix some problems.")

args = parser.parse_args()

num_errors = 0

if args.files:
for filepath in args.files:
num_errors += lint_file(filepath)
num_errors += lint_file(filepath, args)
else:
script_dirpath = os.path.dirname(os.path.realpath(__file__))
root_dirpath = os.path.abspath(f"{script_dirpath}/..")
Expand All @@ -154,6 +302,7 @@ def main() -> None:

exclude_paths = {
"./CODE_STYLE.md",
"./examples/rust/objectron/src/objectron.rs", # auto-generated
"./scripts/lint.py", # we contain all the patterns we are linting against
"./web_viewer/re_viewer_debug.js", # auto-generated by wasm_bindgen
"./web_viewer/re_viewer.js", # auto-generated by wasm_bindgen
Expand All @@ -166,13 +315,13 @@ def main() -> None:
if extension in extensions:
filepath = os.path.join(root, filename)
if filepath not in exclude_paths:
num_errors += lint_file(filepath)
num_errors += lint_file(filepath, args)

if num_errors == 0:
print("lint.py finished without error")
print(f"{sys.argv[0]} finished without error")
sys.exit(0)
else:
print(f"{num_errors} errors.")
print(f"{sys.argv[0]} found {num_errors} errors.")
sys.exit(1)


Expand Down