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

feat(rome_js_analyze): add options to noRestrictedGlobals #4723

Merged
merged 2 commits into from
Jul 24, 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
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 @@ -42,4 +42,4 @@ similar = "2.1.0"
tests_macros = { 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