feat(biome_js_analyze): noRestrictedImports - Added option to allow import from specific locations

[ziongh]

Summary

Sometimes we want to forbid the use of a module in the application but allow in specific places.

One great example of this is when we want to define a linear folder structure and enforce that a folder should not reference the folders above it, but allow referencing all the folders below.

Test Plan

{
    "noRestrictedImports": {
        "options": {
            "paths": {
                "@/features": {
                   "message": "Using lodash is not encouraged",
                   "allowedFrom": ["./src/routes"]
                },
                "underscore": {
                   "message": "Using underscore is not encouraged",
                }
            }
        }
    }
}

In the example above we would be able to allow using @/features module from within ./src/routes but not from other places.

[ematipico]

Wouldn't you achieve the same result by using an override where you turn off/change the rule for src/routes?

[codspeed-hq]

CodSpeed Performance Report

Merging #2977 will not alter performance

_{Comparing ziongh:main (aa2f276) with main (5fe2f68)}

Summary

✅ 90 untouched benchmarks

[ziongh]

About using the override option. that is a possibility for the example I given. However, We would need to replicate all the other options of this rule for each folder of each override (e.g.: block the use of loadesh, etc. for each foder).

And I realized that the implementation that was used forced the path defined to be exactly the imported module. For instance, imagine we have a folder ./src/features, when we do import sampleFunction from '@/features/utils/helper' the rule ´@/features´would not stop this import.

So I've added a new option: includeAllSubmodules.

now we can define:

"noRestrictedImports": {
  "level": "error",
  "options": {
    "paths": {
      "@/features": {
        "message": "It is not allowed to reference @/features from here.",
        "includeAllSubmodules": true,
        "allowedFrom": ["./src/routes"]
      }
    }
  }
}

This way all submodules will also be forbidden (eg.: '@/features/utils/helper')

[ematipico]

A couple of things on this line:

• .contains can create false positives. For example, if your configuration is ./src/something.js, a path like ../src/something.js will pass, which is not what you want, right?
• I don't think we need to allocate a String (via .to_string) to do a simple check

[ziongh]

About the the allocation of the .to_string, I can use:

file_path.to_string_lossy().matches(allowed).count() > 0

And about the false positives, it would be nice to allow something like "./src/**/tests" or other patterns. To allow this, I would need to know the working directory root. because file_path returns the complete path, and to use the "./" notation as you pointed, I'd need to convert it. Is there any helpers to do that?

[ematipico]

Can you please document each new option that you created? The doc documents well with used for the JSON schema, and we want to document everything that is public.

[ematipico]

The changes that you applied are breaking changes, which is fine because this rule is a nursery. However, I still think we should provide the old option. Not all users will need to provide an object, and the might want to provide only a message.

Also, the new options that you added are not documented in the main doc comment. Could you please add more text to describe them?

[ziongh]

About the Option to pass only a message (string) besides the new option that needs an Object, I think It would require a Union Type.

However, a union type cannot be used in Box<T> as far as I've tried. I'm not very familiar with rust...

One option is to create a new lint rule all together, but i think there must be a way. Can you point me in the correct direction? thanks.

[ematipico]

We already have options that are enum. You can copy this code:

biome/crates/biome_configuration/src/parse/json/linter.rs

Lines 65 to 129 in 9e4feb6

	impl<T: Deserializable + Default> Deserializable for RuleConfiguration<T> {
	fn deserialize(
	value: &impl DeserializableValue,
	rule_name: &str,
	diagnostics: &mut Vec<DeserializationDiagnostic>,
	) -> Option<Self> {
	value.deserialize(
	RuleConfigurationVisitor(PhantomData),
	rule_name,
	diagnostics,
	)
	}
	}
	struct RuleConfigurationVisitor<T>(PhantomData<T>);
	impl<T: Deserializable + Default> DeserializationVisitor for RuleConfigurationVisitor<T> {
	type Output = RuleConfiguration<T>;
	const EXPECTED_TYPE: VisitableType = VisitableType::STR.union(VisitableType::MAP);
	fn visit_str(
	self,
	value: Text,
	range: TextRange,
	_rule_name: &str,
	diagnostics: &mut Vec<DeserializationDiagnostic>,
	) -> Option<Self::Output> {
	RulePlainConfiguration::deserialize_from_str(value, range, diagnostics)
	.map(RuleConfiguration::Plain)
	}
	fn visit_map(
	self,
	members: impl Iterator<Item = Option<(impl DeserializableValue, impl DeserializableValue)>>,
	_range: biome_rowan::TextRange,
	rule_name: &str,
	diagnostics: &mut Vec<DeserializationDiagnostic>,
	) -> Option<Self::Output> {
	const ALLOWED_KEYS: &[&str] = &["level", "options"];
	let mut result = RuleWithOptions::default();
	for (key, value) in members.flatten() {
	let Some(key_text) = Text::deserialize(&key, "", diagnostics) else {
	continue;
	};
	match key_text.text() {
	"level" => {
	result.level = Deserializable::deserialize(&value, &key_text, diagnostics)?;
	}
	"options" => {
	if let Some(options) =
	Deserializable::deserialize(&value, rule_name, diagnostics)
	{
	result.options = options;
	}
	}
	unknown_key => diagnostics.push(DeserializationDiagnostic::new_unknown_key(
	unknown_key,
	key.range(),
	ALLOWED_KEYS,
	)),
	}
	}
	Some(RuleConfiguration::WithOptions(result))
	}
	}

It is how we deserialize the configuration of a lint rule, where it can be "off" or an object.