Skip to content

Commit

Permalink
- Switch to clippy::pedantic and cleanup a number of issues.
Browse files Browse the repository at this point in the history
 - update workflow to run clippy
  • Loading branch information
evaneaston committed May 6, 2024
1 parent 5d37f0a commit 7352e33
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 71 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@ jobs:
- run: cargo test

format:
name: Format Check
name: Format Check And Linting
needs: pre_ci
runs-on: ubuntu-latest
if: github.event_name != 'pull_request'
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- run: cargo fmt -- --check
- run: cargo clippy

# outdated:
# name: Outdated
Expand Down
2 changes: 1 addition & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
set -euo pipefail

cargo fmt -- --check
cargo clippy -- -D warnings
cargo clippy
cargo test
cargo doc
19 changes: 12 additions & 7 deletions client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ pub struct Client {

impl Client {
/// Create a new client using the supplied options.
/// Returns an error if it fails because of invalid options.
///
/// # Errors
/// Returns an error if invalid [`ClientOptions`] are provided.
pub fn new(options: ClientOptions) -> Result<Client, ClientError> {
options.validate()?;

Expand All @@ -50,18 +52,21 @@ impl Client {
})
}

/// Fetch the weather for the provided [Query].
/// Fetch the weather for the provided [`Query`].
///
/// # Errors
/// May fail for a variety of reasons, See [`ApiCallError`].
pub async fn fetch_weather(&self, query: &dyn Query) -> Result<CurrentWeather, ApiCallError> {
let url = self.url_for(query)?;
let query_url = self.url_for(query)?;

let uri = match Uri::from_str(url.as_str()) {
let uri = match Uri::from_str(query_url.as_str()) {
Ok(u) => Ok(u),
Err(invalid_uri) => Err(ApiCallError::ErrorFormingUri(invalid_uri)),
}?;

debug!(
"Fetch weather at URL {}",
self.options.mask_api_key_if_present(url.as_str())
self.options.mask_api_key_if_present(query_url.as_str())
);

match self.http_client.get(uri).await {
Expand All @@ -74,7 +79,7 @@ impl Client {
}
Err(error) => Err(ApiCallError::HttpError {
error,
url: self.options.mask_api_key_if_present(url.as_str()),
url: self.options.mask_api_key_if_present(query_url.as_str()),
}),
}
}
Expand Down Expand Up @@ -111,7 +116,7 @@ impl Client {
async fn handle_non_200_response(&self, response_body: Response<Incoming>, sc: &StatusCode) -> ApiCallError {
let rb = match response_body_as_str(response_body).await {
Ok(rb) => rb,
Err(error) => format!("Error obtaining response body {:?}", error),
Err(error) => format!("Error obtaining response body {error:?}"),
};
ApiCallError::InvalidResponsStatus { status: *sc, body: rb }
}
Expand Down
14 changes: 4 additions & 10 deletions client/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
#![deny(clippy::pedantic, clippy::cargo)]
#![warn(clippy::perf, clippy::complexity, clippy::multiple_crate_versions)]
#![allow(clippy::module_name_repetitions, clippy::must_use_candidate)]

//!
//! ## Get An API Key
//!
Expand All @@ -17,16 +21,6 @@
#![doc = include_str!("../examples/get_multiple_readings.rs")]
//! ```
#![deny(clippy::all, clippy::missing_panics_doc)]
#![warn(
rustdoc::broken_intra_doc_links,
clippy::cargo,
clippy::perf,
clippy::complexity,
//missing_docs,
clippy::multiple_crate_versions,
)]

mod client;
pub mod error;
pub mod models;
Expand Down
14 changes: 8 additions & 6 deletions client/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub struct City {
pub display_name: Option<String>,
}
impl City {
/// Create an instance with just name and country_code
/// Create an instance with just `name` and `country_code`
pub fn new(name: &str, country_code: &str) -> City {
City {
name: name.to_string(),
Expand All @@ -61,7 +61,7 @@ impl City {
impl Display for City {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match &self.display_name {
Some(display_name) => write!(f, "{}", display_name),
Some(display_name) => write!(f, "{display_name}"),
None => write!(f, "{}, {}", self.name, self.country_code),
}
}
Expand All @@ -84,7 +84,7 @@ impl CityId {
impl Display for CityId {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match &self.display_name {
Some(display_name) => write!(f, "{}", display_name),
Some(display_name) => write!(f, "{display_name}"),
None => write!(f, "{}", self.id),
}
}
Expand Down Expand Up @@ -113,13 +113,15 @@ impl Coord {
impl Display for Coord {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match &self.display_name {
Some(display_name) => write!(f, "{}", display_name),
Some(display_name) => write!(f, "{display_name}"),
None => write!(f, "lat={}, lon={}", self.lat, self.lon),
}
}
}

/// Main structure for responses from the free OpenWeatherMap API.
/// Main structure for responses from the free `OpenWeatherMap` API
///
/// See their API response documenation [here](https://openweathermap.org/current#fields_json).
#[derive(Debug, Deserialize)]
pub struct CurrentWeather {
/// City geo location, longitude
Expand All @@ -142,7 +144,7 @@ pub struct CurrentWeather {
pub rain: Option<PrecipVolume>,
/// Recent snow volume
pub snow: Option<PrecipVolume>,
/// Time of data calculation, unix, UTC
/// Time of data calculation, unix, UTC (in seconds)
pub dt: i64,
/// See [Sys]
pub sys: Sys,
Expand Down
15 changes: 9 additions & 6 deletions client/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl ClientOptions {
pub fn default_api_key() -> String {
match std::env::var("API_KEY") {
Ok(api_key) => api_key,
Err(_) => "".to_string(),
Err(_) => String::new(),
}
}

Expand All @@ -39,7 +39,7 @@ impl ClientOptions {
"en".to_string()
}

/// Defaults to [UnitSystem::Metric]
/// Defaults to [`UnitSystem::Metric`]
pub fn default_units() -> UnitSystem {
UnitSystem::Metric
}
Expand All @@ -49,7 +49,10 @@ impl ClientOptions {
mask(&self.api_key)
}

/// Ensures an api_key is provided
/// Ensures an `api_key` is provided
///
/// # Errors
/// Errors that cannot be validated during input parsing.
pub fn validate(&self) -> Result<(), InvalidOptionsError> {
if self.api_key.is_empty() {
return Err(InvalidOptionsError {
Expand All @@ -60,14 +63,14 @@ impl ClientOptions {
Ok(())
}

/// Take an arbitrary string that might have the self.api_key in it and returns a string with that all occurrences of the key masked.
/// Take an arbitrary string that might have the `self.api_key` in it and returns a string with that all occurrences of the key masked.
pub fn mask_api_key_if_present(&self, any_string: &str) -> String {
any_string.replace(&self.api_key, &self.masked_api_key())
}
}

impl Default for ClientOptions {
/// Useful for creating options with an API Key specified in the environment variable API_KEY and with defaults of "en", Metric, 1 call/sercond.
/// Useful for creating options with an API Key specified in the environment variable `API_KEY` and with defaults of "en", Metric, 1 call/sercond.
fn default() -> Self {
Self {
api_key: Self::default_api_key(),
Expand Down Expand Up @@ -153,7 +156,7 @@ units: imperial
};
assert_eq!(options.api_key, "PLAINTEXT_API_KEY");

let debug = format!("{:?}", options);
let debug = format!("{options:?}");
assert!(!debug.contains("PLAINTEXT"));
assert!(debug.contains("PLA****"));
}
Expand Down
6 changes: 3 additions & 3 deletions client/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use super::models::{City, CityId, Coord};
pub type QueryParameter = (&'static str, String);
pub type QueryParameters = Vec<QueryParameter>;

/// Abstraction of a query against the OpenWeatherMap API.
/// Abstraction of a query against the `OpenWeatherMap` API.
pub trait Query: fmt::Debug + fmt::Display + Send + Sync {
/// Used in logging and included as a label in metrics published by openweathermap_exporter
/// Used in logging and included as a label in metrics published by `openweathermap_exporter`
fn get_display_name(&self) -> &Option<String>;
/// Query parameters and values that must be added to API call URL.
fn query_params(&self) -> QueryParameters;
Expand Down Expand Up @@ -89,7 +89,7 @@ mod tests {
fn city_id_query() -> (CityId, Vec<QueryParameter>) {
(
CityId {
id: 3665202,
id: 3_665_202,
display_name: None,
},
vec![("id", "3665202".to_owned())],
Expand Down
24 changes: 13 additions & 11 deletions exporter/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl ListenOptions {
}
}
Err(VarError::NotPresent) => default_address(),
Err(VarError::NotUnicode(e)) => panic!("Unable to load environment variable {env_var}. {:?}", e),
Err(VarError::NotUnicode(e)) => panic!("Unable to load environment variable {env_var}. {e:?}"),
}
}

Expand All @@ -71,7 +71,7 @@ impl ListenOptions {
}
}
Err(VarError::NotPresent) => default_port,
Err(VarError::NotUnicode(e)) => panic!("Unable to load environment variable {env_var}. {:?}", e),
Err(VarError::NotUnicode(e)) => panic!("Unable to load environment variable {env_var}. {e:?}"),
}
}
}
Expand Down Expand Up @@ -126,12 +126,12 @@ impl ExporterConfig {
///
/// Searches for the first file from the following list. Once found stops, and loads it.
///
/// * ./owm_exporter.yaml
/// * ./owm_exporter.yml
/// * ./owm_exporter.json
/// * ~/owm_exporter.yaml
/// * ~/owm_exporter.yml
/// * ~/owm_exporter.json
/// * `./owm_exporter.yaml`
/// * `./owm_exporter.yml`
/// * `./owm_exporter.json`
/// * `~/owm_exporter.yaml`
/// * `~/owm_exporter.yml`
/// * `~/owm_exporter.json`
///
/// # Config File Format
///
Expand All @@ -142,6 +142,8 @@ impl ExporterConfig {
/// ```rust
/// let config = Config::load();
/// ```
/// # Errors
/// If the file is missing, cannot be read, contains a syntax error contains invalid values.
///
pub fn load() -> Result<ExporterConfig, ExporterError> {
let path = Self::find_config_file()?;
Expand Down Expand Up @@ -180,7 +182,7 @@ impl ExporterConfig {
None => Err(ExporterError::ConfigNotFound {
message: format!(
"Could not locate any of the following config files {}.",
join_paths(candidate_files, ", ")
join_paths(&candidate_files, ", ")
),
}),
}
Expand All @@ -202,7 +204,7 @@ impl ExporterConfig {
}

match self.owm.validate() {
Ok(_) => Ok(()),
Ok(()) => Ok(()),
Err(e) => Err(ExporterError::ConfigValidationError {
message: "Owm Client Validation error".to_string(),
error: Some(e),
Expand Down Expand Up @@ -235,7 +237,7 @@ fn read_from_path(path: &PathBuf) -> Result<String, ExporterError> {
}
}

fn join_paths(pbs: Vec<PathBuf>, separator: &str) -> String {
fn join_paths(pbs: &[PathBuf], separator: &str) -> String {
pbs.iter()
.map(|pb| pb.to_string_lossy().to_string())
.collect::<Vec<String>>()
Expand Down
Loading

0 comments on commit 7352e33

Please sign in to comment.