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
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Jul 23, 2023
1 parent 9bc3630 commit ed7ffe9
Show file tree
Hide file tree
Showing 40 changed files with 382 additions and 129 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.

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"]
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;
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,7 +6,7 @@ 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(
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
31 changes: 29 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, RestrictedGlobals,
};
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), hide)] RestrictedGlobals),
/// 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(),
_ => RestrictedGlobals::default(),
};
RuleOptions::new(options)
}
// TODO: review error
_ => panic!("This rule {:?} doesn't have options", rule_key),
}
Expand Down Expand Up @@ -107,13 +119,19 @@ 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 = RestrictedGlobals::default();
options.visit_map(key.syntax(), value.syntax(), diagnostics)?;
*self = PossibleOptions::RestrictedGlobals(options);
}

_ => (),
}
}
Expand Down Expand Up @@ -157,6 +175,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, StringSet, 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,70 @@ 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 RestrictedGlobals {
/// A list of names that should trigger the rule
#[bpaf(hide)]
#[serde(skip_serializing_if = "StringSet::is_empty")]
denied_globals: StringSet,
}

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

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

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

impl VisitJsonNode for RestrictedGlobals {}
impl VisitNode<JsonLanguage> for RestrictedGlobals {
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();
match name_text {
"deniedGlobals" => {
self.denied_globals =
StringSet::new(self.map_to_index_set_string(&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 = RestrictedGlobals;

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 +153,18 @@ impl Rule for NoRestrictedGlobals {
};
let token = token.ok()?;
let text = token.text_trimmed();
is_restricted(text, binding).map(|text| (token.text_trimmed_range(), text))
is_restricted(
text,
binding,
options
.denied_globals
.index_set()
.iter()
.map(AsRef::as_ref)
.collect::<Vec<_>>()
.as_slice(),
)
.map(|text| (token.text_trimmed_range(), text))
})
.collect()
}
Expand All @@ -95,8 +185,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($);
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: additionalGlobal.js
---
# Input
```js
console.log($);

```

# Diagnostics
```
additionalGlobal.js:1:13 lint/style/noRestrictedGlobals ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Do not use the global variable $.
> 1 │ console.log($);
│ ^
2 │
i Use a local variable instead.
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"$schema": "../../../../../../npm/rome/configuration_schema.json",
"linter": {
"rules": {
"style": {
"noRestrictedGlobals": {
"level": "error",
"options": {
"deniedGlobals": ["$"]
}
}
}
}
}
}
4 changes: 2 additions & 2 deletions crates/rome_service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ serde_json = { workspace = true, features = ["raw_value"] }
tracing = { workspace = true, features = ["attributes"] }

[features]
schemars = [
schema = [
"dep:schemars",
"rome_js_analyze/schemars",
"rome_js_analyze/schema",
"rome_formatter/serde",
"rome_js_factory",
"rome_text_edit/schemars",
Expand Down
6 changes: 3 additions & 3 deletions crates/rome_service/src/configuration/formatter.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use crate::configuration::merge::MergeWith;
use crate::configuration::string_set::StringSet;
use crate::settings::FormatSettings;
use crate::{ConfigurationDiagnostic, MatchOptions, Matcher, WorkspaceError};
use bpaf::Bpaf;
use rome_deserialize::StringSet;
use rome_formatter::{IndentStyle, LineWidth};
use serde::{Deserialize, Serialize};
use std::str::FromStr;

/// Options applied to the formatter
#[derive(Deserialize, Serialize, Debug, Eq, PartialEq, Clone, Bpaf)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase", default, deny_unknown_fields)]
pub struct FormatterConfiguration {
// if `false`, it disables the feature. `true` by default
Expand Down Expand Up @@ -153,7 +153,7 @@ where
}

#[derive(Deserialize, Serialize, Debug, Eq, PartialEq, Clone, Default)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase")]
pub enum PlainIndentStyle {
/// Tab
Expand Down
Loading

0 comments on commit ed7ffe9

Please sign in to comment.