Skip to content

Commit

Permalink
Unused endpoints should produce a lint warning (#116)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahl authored Jun 9, 2021
1 parent e0201cf commit df3b363
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.5.1\...HEAD[Full list of co
=== Breaking Changes

* https://github.com/oxidecomputer/dropshot/pull/100[#100] The type used for the "limit" argument for paginated resources has changed. This limit refers to the number of items that an HTTP client can ask for in a single request to a paginated endpoint. The limit is now 4294967295, where it may have previously been larger. This is not expected to affect consumers because this limit is far larger than practical. For details, see #100.
* https://github.com/oxidecomputer/dropshot/pull/116[#116] Unused, non-`pub` endpoints from the `#[endpoint { ... }]` macro now produce a lint warning. This is *technically* a breaking change for those who may have had unused endpoints and compiled with `#[deny(warning)]` or `#[deny(dead_code)]` thus implicitly relying on the *absence* of a warning about the endpoint being unused.

== 0.5.1 (released 2021-03-18)

Expand Down
29 changes: 29 additions & 0 deletions dropshot/tests/fail/unused_endpoint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2021 Oxide Computer Company

// For whatever reason, trybuild overrides the default disposition for the
// dead_code lint from "warn" to "allow" so we explicitly deny it here.
#![deny(dead_code)]

use dropshot::endpoint;
use dropshot::HttpError;
use dropshot::HttpResponseOk;
use dropshot::RequestContext;
use std::sync::Arc;

// At some point we'd expect to see code like:
// ```
// let mut api = ApiDescription::new();
// api.register(unused_endpoint).unwrap();
// ```
// Defining but not using the endpoint should cause a warning.
#[endpoint {
method = GET,
path = "/test",
}]
async fn unused_endpoint(
_rqctx: Arc<RequestContext<()>>,
) -> Result<HttpResponseOk<()>, HttpError> {
Ok(HttpResponseOk(()))
}

fn main() {}
11 changes: 11 additions & 0 deletions dropshot/tests/fail/unused_endpoint.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: constant is never used: `unused_endpoint`
--> $DIR/unused_endpoint.rs:23:10
|
23 | async fn unused_endpoint(
| ^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/unused_endpoint.rs:5:9
|
5 | #![deny(dead_code)]
| ^^^^^^^^^
14 changes: 12 additions & 2 deletions dropshot_endpoint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,18 @@ fn do_endpoint(
})
.collect::<Vec<_>>();

// For reasons that are not well understood unused constants that use the
// (default) call_site() Span do not trigger the dead_code lint. Because
// defining but not using an endpoint is likely a programming error, we
// want to be sure to have the compiler flag this. We force this by using
// the span from the name of the function to which this macro was applied.
let span = ast.sig.ident.span();
let const_struct = quote_spanned! {span=>
#visibility const #name: #name = #name {};
};

// The final TokenStream returned will have a few components that reference
// `#name`, the name of the method to which this macro was applied...
// `#name`, the name of the function to which this macro was applied...
let stream = quote! {
#(#checks)*

Expand All @@ -224,7 +234,7 @@ fn do_endpoint(
// ... a constant of type `#name` whose identifier is also #name
#[allow(non_upper_case_globals, missing_docs)]
#description_doc_comment
#visibility const #name: #name = #name {};
#const_struct

// ... an impl of `From<#name>` for ApiEndpoint that allows the constant
// `#name` to be passed into `ApiDescription::register()`
Expand Down

0 comments on commit df3b363

Please sign in to comment.