Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_js_analyze): add options to noRestrictedGlobals (#4723)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Jul 24, 2023
1 parent a624cbe commit 1dcd62b
Show file tree
Hide file tree
Showing 43 changed files with 423 additions and 132 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/rome_cli/src/vcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ use crate::cli_options::CliOptions;
use crate::diagnostics::{DisabledVcs, NoVcsFolderFound};
use crate::{CliDiagnostic, CliSession};
use rome_console::{markup, ConsoleExt};
use rome_deserialize::StringSet;
use rome_diagnostics::PrintDiagnostic;
use rome_service::configuration::vcs::{VcsClientKind, VcsConfiguration};
use rome_service::configuration::{FilesConfiguration, StringSet};
use rome_service::configuration::FilesConfiguration;
use rome_service::{Configuration, WorkspaceError};
use std::path::PathBuf;

Expand Down
6 changes: 5 additions & 1 deletion crates/rome_deserialize/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ version = "0.0.0"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
indexmap = { workspace = true }
indexmap = { workspace = true, features = ["serde"] }
rome_console = { workspace = true }
rome_diagnostics = { workspace = true }
rome_json_parser = { workspace = true }
rome_json_syntax = { workspace = true }
rome_rowan = { workspace = true }
schemars = { workspace = true, optional = true }
serde = { workspace = true }
serde_json = { workspace = true }
tracing = { workspace = true }

[features]
schema = ["schemars", "schemars/indexmap"]
43 changes: 43 additions & 0 deletions crates/rome_deserialize/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,49 @@ pub trait VisitJsonNode: VisitNode<JsonLanguage> {
Some(elements)
}

/// It attempts to map a [AnyJsonValue] to a [Vec] of [String].
///
/// ## Errors
///
/// The function emit diagnostics if:
/// - `value` can't be cast to [JsonArrayValue]
/// - any element of the of the array can't be cast to [JsonStringValue]
fn map_to_array_of_strings(
&self,
value: &AnyJsonValue,
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Vec<String>> {
let array = JsonArrayValue::cast_ref(value.syntax()).or_else(|| {
diagnostics.push(DeserializationDiagnostic::new_incorrect_type_for_value(
name,
"array",
value.range(),
));
None
})?;
let mut elements = Vec::new();
if array.elements().is_empty() {
return None;
}
for element in array.elements() {
let element = element.ok()?;
match element {
AnyJsonValue::JsonStringValue(value) => {
elements.push(value.inner_string_text().ok()?.to_string());
}
_ => {
diagnostics.push(DeserializationDiagnostic::new_incorrect_type(
"string",
element.range(),
));
}
}
}

Some(elements)
}

/// It attempts to map [AnyJsonValue] to a generic map.
///
/// Use this function when the value of your member is another object, and this object
Expand Down
3 changes: 3 additions & 0 deletions crates/rome_deserialize/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ mod diagnostics;
mod visitor;

pub mod json;
pub mod string_set;

pub use diagnostics::{DeserializationAdvice, DeserializationDiagnostic};
use rome_diagnostics::Error;
pub use string_set::{deserialize_string_set, serialize_string_set, StringSet};
pub use visitor::VisitNode;

/// A small type to interrogate the result of a JSON deserialization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use std::marker::PhantomData;
use std::str::FromStr;

#[derive(Default, Debug, Deserialize, Serialize, Clone, Eq, PartialEq)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct StringSet(
#[serde(
deserialize_with = "crate::deserialize_string_set",
serialize_with = "crate::serialize_string_set"
)]
IndexSet<String>,
pub IndexSet<String>,
);

impl StringSet {
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/src/adapters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl Diagnostic for IoError {
}
}

/// Implements [Diagnostic] over for [pico_args::Error].
/// Implements [Diagnostic] over for [bpaf::ParseFailure].
#[derive(Debug)]
pub struct BpafError {
error: bpaf::ParseFailure,
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_analyze/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ tests_macros = { workspace = true }
rome_test_utils = { workspace = true }

[features]
schemars = ["dep:schemars"]
schema = ["schemars", "rome_deserialize/schema"]
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ pub struct ComplexityScore {

/// Options for the rule `noNestedModuleImports`.
#[derive(Deserialize, Serialize, Debug, Clone, Bpaf)]
#[cfg_attr(feature = "schemars", derive(JsonSchema))]
#[cfg_attr(feature = "schema", derive(JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct ComplexityOptions {
/// The maximum complexity score that we allow. Anything higher is considered excessive.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,18 @@ declare_rule! {
/// ```js,expect_diagnostic
/// // Attempt to import from `foo.js` from outside its `sub` module.
/// import { fooPackageVariable } from "./sub/foo.js";
///
/// ```
/// ```js,expect_diagnostic
/// // Attempt to import from `bar.ts` from outside its `aunt` module.
/// import { barPackageVariable } from "../aunt/bar.ts";
/// ```
///
/// ```js,expect_diagnostic
/// // Assumed to resolve to a JS/TS file.
/// import { fooPackageVariable } from "./sub/foo";
/// ```
///
/// ```js,expect_diagnostic
/// // If the `sub/foo` module is inaccessible, so is its index file.
/// import { fooPackageVariable } from "./sub/foo/index.js";
/// ```
Expand Down
34 changes: 32 additions & 2 deletions crates/rome_js_analyze/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ use crate::semantic_analyzers::nursery::use_exhaustive_dependencies::{
use crate::semantic_analyzers::nursery::use_naming_convention::{
naming_convention_options, NamingConventionOptions,
};
use crate::semantic_analyzers::style::no_restricted_globals::{
restricted_globals_options, RestrictedGlobalsOptions,
};
use bpaf::Bpaf;
use rome_analyze::options::RuleOptions;
use rome_analyze::RuleKey;
Expand All @@ -29,6 +32,8 @@ pub enum PossibleOptions {
Hooks(#[bpaf(external(hooks_options), hide)] HooksOptions),
/// Options for `useNamingConvention` rule
NamingConvention(#[bpaf(external(naming_convention_options), hide)] NamingConventionOptions),
/// Options for `noRestrictedGlobals` rule
RestrictedGlobals(#[bpaf(external(restricted_globals_options), hide)] RestrictedGlobalsOptions),
/// No options available
#[default]
NoOptions,
Expand Down Expand Up @@ -61,11 +66,18 @@ impl PossibleOptions {
}
"useNamingConvention" => {
let options = match self {
PossibleOptions::NamingConvention(options) => *options,
PossibleOptions::NamingConvention(options) => options.clone(),
_ => NamingConventionOptions::default(),
};
RuleOptions::new(options)
}
"noRestrictedGlobals" => {
let options = match self {
PossibleOptions::RestrictedGlobals(options) => options.clone(),
_ => RestrictedGlobalsOptions::default(),
};
RuleOptions::new(options)
}
// TODO: review error
_ => panic!("This rule {:?} doesn't have options", rule_key),
}
Expand Down Expand Up @@ -107,13 +119,22 @@ impl PossibleOptions {
}
"strictCase" | "enumMemberCase" => {
let mut options = match self {
PossibleOptions::NamingConvention(options) => *options,
PossibleOptions::NamingConvention(options) => options.clone(),
_ => NamingConventionOptions::default(),
};
options.visit_map(key.syntax(), value.syntax(), diagnostics)?;
*self = PossibleOptions::NamingConvention(options);
}

"deniedGlobals" => {
let mut options = match self {
PossibleOptions::RestrictedGlobals(options) => options.clone(),
_ => RestrictedGlobalsOptions::default(),
};
options.visit_map(key.syntax(), value.syntax(), diagnostics)?;
*self = PossibleOptions::RestrictedGlobals(options);
}

_ => (),
}
}
Expand Down Expand Up @@ -157,6 +178,15 @@ impl PossibleOptions {
));
}
}
"noRestrictedGlobals" => {
if !matches!(key_name, "deniedGlobals") {
diagnostics.push(DeserializationDiagnostic::new_unknown_key(
key_name,
node.range(),
&["deniedGlobals"],
));
}
}
_ => {}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ pub(crate) struct State {
}

/// Rule's options.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Serialize, Deserialize, Bpaf)]
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Bpaf)]
#[cfg_attr(feature = "schemars", derive(JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct NamingConventionOptions {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
use crate::semantic_services::SemanticServices;
use bpaf::Bpaf;
use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_deserialize::json::{has_only_known_keys, VisitJsonNode};
use rome_deserialize::{DeserializationDiagnostic, VisitNode};
use rome_js_semantic::{Binding, BindingExtensions};
use rome_js_syntax::{
JsIdentifierAssignment, JsReferenceIdentifier, JsxReferenceIdentifier, TextRange,
};
use rome_rowan::{declare_node_union, AstNode};
use rome_json_syntax::JsonLanguage;
use rome_rowan::{declare_node_union, AstNode, SyntaxNode};
use serde::{Deserialize, Serialize};
use std::str::FromStr;

declare_rule! {
/// This rule allows you to specify global variable names that you don’t want to use in your application.
Expand All @@ -28,6 +34,23 @@ declare_rule! {
/// console.log(event)
/// }
/// ```
/// ## Options
///
/// Use the options to specify additional globals that you want to restrict in your
/// source code.
///
/// ```json
/// {
/// "//": "...",
/// "options": {
/// "deniedGlobals": ["$", "MooTools"]
/// }
/// }
/// ```
///
/// In the example above, the rule will emit a diagnostics if tried to use `$` or `MooTools` without
/// creating a local variable.
///
pub(crate) NoRestrictedGlobals {
version: "0.10.0",
name: "noRestrictedGlobals",
Expand All @@ -41,14 +64,66 @@ declare_node_union! {

const RESTRICTED_GLOBALS: [&str; 2] = ["event", "error"];

/// Options for the rule `noRestrictedGlobals`.
#[derive(Default, Deserialize, Serialize, Debug, Clone, Bpaf)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct RestrictedGlobalsOptions {
/// A list of names that should trigger the rule
#[serde(skip_serializing_if = "Option::is_none")]
#[bpaf(hide, argument::<String>("NUM"), many, optional)]
denied_globals: Option<Vec<String>>,
}

impl RestrictedGlobalsOptions {
pub const KNOWN_KEYS: &'static [&'static str] = &["deniedGlobals"];
}

// Required by [Bpaf].
impl FromStr for RestrictedGlobalsOptions {
type Err = &'static str;

fn from_str(_s: &str) -> Result<Self, Self::Err> {
// WARNING: should not be used.
Ok(Self::default())
}
}

impl VisitJsonNode for RestrictedGlobalsOptions {}
impl VisitNode<JsonLanguage> for RestrictedGlobalsOptions {
fn visit_member_name(
&mut self,
node: &SyntaxNode<JsonLanguage>,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<()> {
has_only_known_keys(node, Self::KNOWN_KEYS, diagnostics)
}

fn visit_map(
&mut self,
key: &SyntaxNode<JsonLanguage>,
value: &SyntaxNode<JsonLanguage>,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<()> {
let (name, value) = self.get_key_and_value(key, value, diagnostics)?;
let name_text = name.text();
if name_text == "deniedGlobals" {
self.denied_globals = self.map_to_array_of_strings(&value, name_text, diagnostics);
}

Some(())
}
}

impl Rule for NoRestrictedGlobals {
type Query = SemanticServices;
type State = (TextRange, String);
type Signals = Vec<Self::State>;
type Options = ();
type Options = RestrictedGlobalsOptions;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let model = ctx.model();
let options = ctx.options();

let unresolved_reference_nodes = model
.all_unresolved_references()
Expand All @@ -74,7 +149,13 @@ impl Rule for NoRestrictedGlobals {
};
let token = token.ok()?;
let text = token.text_trimmed();
is_restricted(text, binding).map(|text| (token.text_trimmed_range(), text))
let denied_globals = if let Some(denied_globals) = options.denied_globals.as_ref() {
denied_globals.iter().map(AsRef::as_ref).collect::<Vec<_>>()
} else {
vec![]
};
is_restricted(text, binding, denied_globals.as_slice())
.map(|text| (token.text_trimmed_range(), text))
})
.collect()
}
Expand All @@ -95,8 +176,8 @@ impl Rule for NoRestrictedGlobals {
}
}

fn is_restricted(name: &str, binding: Option<Binding>) -> Option<String> {
if binding.is_none() && RESTRICTED_GLOBALS.contains(&name) {
fn is_restricted(name: &str, binding: Option<Binding>, denied_globals: &[&str]) -> Option<String> {
if binding.is_none() && (RESTRICTED_GLOBALS.contains(&name) || denied_globals.contains(&name)) {
Some(name.to_string())
} else {
None
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log($);
Loading

0 comments on commit 1dcd62b

Please sign in to comment.