Skip to content

Commit

Permalink
feat: remove ErrorSpan in dependency trait
Browse files Browse the repository at this point in the history
  • Loading branch information
shulaoda committed Sep 9, 2024
1 parent 8134f86 commit b508d0a
Show file tree
Hide file tree
Showing 29 changed files with 103 additions and 109 deletions.
2 changes: 1 addition & 1 deletion crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1607,7 +1607,7 @@ impl ConcatenatedModule {
source_order: dep
.source_order()
.expect("source order should not be empty"),
range_start: dep.span().map(|span| span.start),
range_start: dep.range().map(|range| range.start),
})
})
.collect::<Vec<_>>();
Expand Down
11 changes: 7 additions & 4 deletions crates/rspack_core/src/context_module_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ use rspack_regex::RspackRegex;
use tracing::instrument;

use crate::{
resolve, ContextModule, ContextModuleOptions, DependencyCategory, ModuleExt, ModuleFactory,
ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, RawModule, ResolveArgs,
ResolveOptionsWithDependencyType, ResolveResult, Resolver, ResolverFactory, SharedPluginDriver,
resolve, ContextModule, ContextModuleOptions, DependencyCategory, ErrorSpan, ModuleExt,
ModuleFactory, ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, RawModule,
ResolveArgs, ResolveOptionsWithDependencyType, ResolveResult, Resolver, ResolverFactory,
SharedPluginDriver,
};

#[derive(Clone)]
Expand Down Expand Up @@ -221,7 +222,9 @@ impl ContextModuleFactory {
specifier,
dependency_type: dependency.dependency_type(),
dependency_category: dependency.category(),
span: dependency.span(),
span: dependency
.range()
.map(|range| ErrorSpan::new(range.start, range.end)),
resolve_options: data.resolve_options.clone(),
resolve_to_context: true,
optional: dependency.get_optional(),
Expand Down
5 changes: 3 additions & 2 deletions crates/rspack_core/src/dependency/dependency_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ use swc_core::ecma::atoms::Atom;
use super::dependency_template::AsDependencyTemplate;
use super::module_dependency::*;
use super::ExportsSpec;
use super::RealDependencyLocation;
use super::{DependencyCategory, DependencyId, DependencyType};
use crate::create_exports_object_referenced;
use crate::AsContextDependency;
use crate::ExtendedReferencedExport;
use crate::ImportAttributes;
use crate::ModuleLayer;
use crate::RuntimeSpec;
use crate::{ConnectionState, Context, ErrorSpan, ModuleGraph, UsedByExports};
use crate::{ConnectionState, Context, ModuleGraph, UsedByExports};

#[derive(Debug, Clone, Copy)]
pub enum AffectType {
Expand Down Expand Up @@ -77,7 +78,7 @@ pub trait Dependency:
None
}

fn span(&self) -> Option<ErrorSpan> {
fn range(&self) -> Option<&RealDependencyLocation> {
None
}

Expand Down
4 changes: 3 additions & 1 deletion crates/rspack_core/src/dependency/module_dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ pub trait ModuleDependency: Dependency {
/// This is only intended used to display better diagnostics.
/// So it might not be precise as it is using [crate::Dependency::span] as the default value.
fn source_span(&self) -> Option<ErrorSpan> {
self.span()
self
.range()
.map(|range| ErrorSpan::new(range.start, range.end))
}

// TODO: move to ModuleGraphConnection
Expand Down
6 changes: 3 additions & 3 deletions crates/rspack_plugin_css/src/dependency/compose.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rspack_core::{
AsContextDependency, AsDependencyTemplate, Dependency, DependencyCategory, DependencyId,
DependencyType, ErrorSpan, ModuleDependency, RealDependencyLocation,
DependencyType, ModuleDependency, RealDependencyLocation,
};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -33,8 +33,8 @@ impl Dependency for CssComposeDependency {
&DependencyType::CssCompose
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
8 changes: 4 additions & 4 deletions crates/rspack_plugin_css/src/dependency/import.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rspack_core::{
AsContextDependency, Compilation, Dependency, DependencyCategory, DependencyId,
DependencyTemplate, DependencyType, ErrorSpan, ModuleDependency, RealDependencyLocation,
RuntimeSpec, TemplateContext, TemplateReplaceSource,
DependencyTemplate, DependencyType, ModuleDependency, RealDependencyLocation, RuntimeSpec,
TemplateContext, TemplateReplaceSource,
};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -34,8 +34,8 @@ impl Dependency for CssImportDependency {
&DependencyType::CssImport
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
10 changes: 5 additions & 5 deletions crates/rspack_plugin_css/src/dependency/url.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use rspack_core::{
AsContextDependency, CodeGenerationDataFilename, CodeGenerationDataUrl, Compilation, Dependency,
DependencyCategory, DependencyId, DependencyTemplate, DependencyType, ErrorSpan,
ModuleDependency, ModuleIdentifier, PublicPath, RealDependencyLocation, RuntimeSpec,
TemplateContext, TemplateReplaceSource,
DependencyCategory, DependencyId, DependencyTemplate, DependencyType, ModuleDependency,
ModuleIdentifier, PublicPath, RealDependencyLocation, RuntimeSpec, TemplateContext,
TemplateReplaceSource,
};

use crate::utils::{css_escape_string, AUTO_PUBLIC_PATH_PLACEHOLDER};
Expand Down Expand Up @@ -64,8 +64,8 @@ impl Dependency for CssUrlDependency {
&DependencyType::CssUrl
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
18 changes: 8 additions & 10 deletions crates/rspack_plugin_extract_css/src/css_dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::PathBuf;
use rspack_collections::IdentifierSet;
use rspack_core::{
AffectType, AsContextDependency, AsDependencyTemplate, ConnectionState, Dependency,
DependencyCategory, DependencyId, ModuleDependency, ModuleGraph,
DependencyCategory, DependencyId, ModuleDependency, ModuleGraph, RealDependencyLocation,
};
use rustc_hash::FxHashSet;

Expand All @@ -24,8 +24,9 @@ pub struct CssDependency {
pub(crate) identifier_index: u32,

// determine module's postOrderIndex
pub(crate) order_index: u32,

// @TODO(shulaoda) Does this have any additional side effects?
// pub(crate) order_index: u32,
range: RealDependencyLocation,
resource_identifier: String,
pub(crate) cacheable: bool,
pub(crate) file_dependencies: FxHashSet<PathBuf>,
Expand All @@ -45,7 +46,7 @@ impl CssDependency {
supports: Option<String>,
source_map: Option<String>,
identifier_index: u32,
order_index: u32,
range: RealDependencyLocation,
cacheable: bool,
file_dependencies: FxHashSet<PathBuf>,
context_dependencies: FxHashSet<PathBuf>,
Expand All @@ -63,7 +64,7 @@ impl CssDependency {
supports,
source_map,
identifier_index,
order_index,
range,
resource_identifier,
cacheable,
file_dependencies,
Expand Down Expand Up @@ -107,11 +108,8 @@ impl Dependency for CssDependency {
// it can keep the right order, but Rspack uses HashSet,
// when determining the postOrderIndex, Rspack uses
// dependency span to set correct order
fn span(&self) -> Option<rspack_core::ErrorSpan> {
Some(rspack_core::ErrorSpan {
start: self.order_index,
end: self.order_index + 1,
})
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn get_layer(&self) -> Option<&rspack_core::ModuleLayer> {
Expand Down
4 changes: 2 additions & 2 deletions crates/rspack_plugin_extract_css/src/parser_plugin.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rspack_core::BoxDependency;
use rspack_core::{BoxDependency, RealDependencyLocation};
use rspack_plugin_javascript::{visitors::JavascriptParser, JavascriptParserPlugin};
use rspack_util::fx_hash::FxDashMap;
use serde::Deserialize;
Expand Down Expand Up @@ -60,7 +60,7 @@ impl JavascriptParserPlugin for PluginCssExtractParserPlugin {
supports.clone(),
source_map.clone(),
*identifier_index,
index as u32,
RealDependencyLocation::new(index as u32, (index + 1) as u32),
parser.build_info.cacheable,
parser.build_info.file_dependencies.clone(),
parser.build_info.context_dependencies.clone(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rspack_core::{
};
use rspack_core::{AsContextDependency, Dependency, DependencyCategory};
use rspack_core::{DependencyId, DependencyTemplate};
use rspack_core::{DependencyType, ErrorSpan, ModuleDependency};
use rspack_core::{DependencyType, ModuleDependency};
use rspack_core::{TemplateContext, TemplateReplaceSource};
use swc_core::atoms::Atom;

Expand Down Expand Up @@ -58,8 +58,8 @@ impl Dependency for CommonJsFullRequireDependency {
Some(self.range.to_string())
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn get_referenced_exports(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rspack_core::{module_id, Compilation, RealDependencyLocation, RuntimeSpec};
use rspack_core::{AsContextDependency, Dependency, DependencyCategory};
use rspack_core::{DependencyId, DependencyTemplate};
use rspack_core::{DependencyType, ErrorSpan, ModuleDependency};
use rspack_core::{DependencyType, ModuleDependency};
use rspack_core::{TemplateContext, TemplateReplaceSource};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -47,11 +47,8 @@ impl Dependency for CommonJsRequireDependency {
&DependencyType::CjsRequire
}

fn span(&self) -> Option<ErrorSpan> {
self
.range
.clone()
.map(|range| ErrorSpan::new(range.start, range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
self.range.as_ref()
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rspack_core::{
module_id, AsContextDependency, Compilation, Dependency, DependencyCategory, DependencyId,
DependencyTemplate, DependencyType, ErrorSpan, ExtendedReferencedExport, ModuleDependency,
ModuleGraph, RealDependencyLocation, RuntimeSpec, TemplateContext, TemplateReplaceSource,
DependencyTemplate, DependencyType, ExtendedReferencedExport, ModuleDependency, ModuleGraph,
RealDependencyLocation, RuntimeSpec, TemplateContext, TemplateReplaceSource,
};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -38,8 +38,8 @@ impl Dependency for RequireResolveDependency {
&DependencyType::RequireResolve
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn get_referenced_exports(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rspack_core::{
};
use rspack_core::{ContextOptions, Dependency, TemplateReplaceSource};
use rspack_core::{DependencyCategory, DependencyId, DependencyTemplate};
use rspack_core::{DependencyType, ErrorSpan, TemplateContext};
use rspack_core::{DependencyType, TemplateContext};

use super::{
context_dependency_template_as_require_call, create_resource_identifier_for_context_dependency,
Expand Down Expand Up @@ -51,8 +51,8 @@ impl Dependency for CommonJsRequireContextDependency {
&DependencyType::CommonJSRequireContext
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rspack_core::{
};
use rspack_core::{ContextOptions, Dependency, TemplateReplaceSource};
use rspack_core::{DependencyCategory, DependencyId, DependencyTemplate};
use rspack_core::{DependencyType, ErrorSpan, TemplateContext};
use rspack_core::{DependencyType, TemplateContext};

use super::{
context_dependency_template_as_require_call, create_resource_identifier_for_context_dependency,
Expand Down Expand Up @@ -51,8 +51,8 @@ impl Dependency for ImportContextDependency {
&DependencyType::ImportContext
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rspack_core::{
RuntimeSpec,
};
use rspack_core::{ContextOptions, Dependency, DependencyCategory, DependencyId};
use rspack_core::{DependencyTemplate, DependencyType, ErrorSpan};
use rspack_core::{DependencyTemplate, DependencyType};
use rspack_core::{TemplateContext, TemplateReplaceSource};

use super::create_resource_identifier_for_context_dependency;
Expand Down Expand Up @@ -43,8 +43,8 @@ impl Dependency for ImportMetaContextDependency {
&DependencyType::ImportMetaContext
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rspack_core::{
RuntimeSpec,
};
use rspack_core::{ContextOptions, Dependency, DependencyCategory, DependencyId};
use rspack_core::{DependencyTemplate, DependencyType, ErrorSpan};
use rspack_core::{DependencyTemplate, DependencyType};
use rspack_core::{TemplateContext, TemplateReplaceSource};

use super::create_resource_identifier_for_context_dependency;
Expand Down Expand Up @@ -43,8 +43,8 @@ impl Dependency for RequireContextDependency {
&DependencyType::RequireContext
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use rspack_core::{
process_export_info, property_access, property_name, string_of_used_name, AsContextDependency,
Compilation, ConditionalInitFragment, ConnectionState, Dependency, DependencyCategory,
DependencyCondition, DependencyConditionFn, DependencyId, DependencyTemplate, DependencyType,
ErrorSpan, ExportInfo, ExportInfoProvided, ExportNameOrSpec, ExportPresenceMode, ExportSpec,
ExportsInfo, ExportsOfExportsSpec, ExportsSpec, ExportsType, ExtendedReferencedExport,
ExportInfo, ExportInfoProvided, ExportNameOrSpec, ExportPresenceMode, ExportSpec, ExportsInfo,
ExportsOfExportsSpec, ExportsSpec, ExportsType, ExtendedReferencedExport,
HarmonyExportInitFragment, ImportAttributes, InitFragmentExt, InitFragmentKey, InitFragmentStage,
JavascriptParserOptions, ModuleDependency, ModuleGraph, ModuleIdentifier, NormalInitFragment,
RealDependencyLocation, RuntimeCondition, RuntimeGlobals, RuntimeSpec, Template, TemplateContext,
Expand Down Expand Up @@ -866,7 +866,7 @@ impl HarmonyExportImportedSpecifierDependency {
let parent_module_identifier = module_graph
.get_parent_module(&self.id)
.expect("should have parent module for dependency");
let mut diagnostic = if let Some(span) = self.span()
let mut diagnostic = if let Some(span) = self.range()
&& let Some(parent_module) = module_graph.module_by_identifier(parent_module_identifier)
&& let Some(source) = parent_module.original_source().map(|s| s.source())
{
Expand Down Expand Up @@ -1054,8 +1054,8 @@ impl Dependency for HarmonyExportImportedSpecifierDependency {
Some(self.range.to_string())
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn category(&self) -> &DependencyCategory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ pub fn harmony_import_dependency_get_linking_error<T: ModuleDependency>(
} else {
(Severity::Warning, "HarmonyLinkingWarning")
};
let mut diagnostic = if let Some(span) = module_dependency.span()
let mut diagnostic = if let Some(span) = module_dependency.range()
&& let Some(source) = parent_module.original_source().map(|s| s.source())
{
Diagnostic::from(
Expand Down Expand Up @@ -380,8 +380,8 @@ impl Dependency for HarmonyImportSideEffectDependency {
Some(self.range.to_string())
}

fn span(&self) -> Option<ErrorSpan> {
Some(ErrorSpan::new(self.range.start, self.range.end))
fn range(&self) -> Option<&RealDependencyLocation> {
Some(&self.range)
}

fn source_order(&self) -> Option<i32> {
Expand Down
Loading

0 comments on commit b508d0a

Please sign in to comment.