Jan 13, 2025 · Dec 24, 2024 · Dec 26, 2024 · Jan 2, 2025 · Jan 2, 2025 · Jan 12, 2025
diff --git a/packages/eslint-plugin/docs/rules/no-shadow.mdx b/packages/eslint-plugin/docs/rules/no-shadow.mdx
 This rule adds the following options:

 ```ts
 type AdditionalHoistOptionEntries = 'types' | 'functions-and-types';

 type HoistOptionEntries =
  | BaseNoShadowHoistOptionEntries
  | AdditionalHoistOptionEntries;

 interface Options extends BaseNoShadowOptions {
  hoist?: HoistOptionEntries;
  ignoreTypeValueShadow?: boolean;
  ignoreFunctionTypeParameterNameValueShadow?: boolean;
 }

 const defaultOptions: Options = {
  ...baseNoShadowDefaultOptions,
  hoist: 'functions-and-types',
  ignoreTypeValueShadow: true,
  ignoreFunctionTypeParameterNameValueShadow: true,
 };
 ```

 ### hoist: `types`

 Examples of incorrect code for the `{ "hoist": "types" }` option:

 ```ts option='{ "hoist": "types" }' showPlaygroundButton
 type Bar<Foo> = 1;
 type Foo = 1;
 ```

 ### hoist: `functions-and-types`

 Examples of incorrect code for the `{ "hoist": "functions-and-types" }` option:

 ```ts option='{ "hoist": "functions-and-types" }' showPlaygroundButton
 // types
 type Bar<Foo> = 1;
 type Foo = 1;

 // functions
 if (true) {
  let b = 6;
 }

 function b() {}
 ```

 ### `ignoreTypeValueShadow`

 {/* insert option description */}
diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts
 {
 allow?:string[];
 builtinGlobals?:boolean;
 hoist?:'all'|'functions'|'never';
 hoist?:'all'|'functions'|'functions-and-types'|'never'|'types';
 ignoreFunctionTypeParameterNameValueShadow?:boolean;
 ignoreOnInitialization?:boolean;
 ignoreTypeValueShadow?:boolean;
 AST_NODE_TYPES.TSConstructorType,
 ]);

 constfunctionsHoistedNodes=newSet([AST_NODE_TYPES.FunctionDeclaration]);

 consttypesHoistedNodes=newSet([
 AST_NODE_TYPES.TSInterfaceDeclaration,
 AST_NODE_TYPES.TSTypeAliasDeclaration,
 ]);

 exportdefaultcreateRule<Options,MessageIds>({
 name:'no-shadow',
 meta:{
 type:'string',
 description:
 'Whether to report shadowing before outer functions or variables are defined.',
 enum:['all','functions','never'],
 enum:['all','functions','functions-and-types','never','types'],
 },
 ignoreFunctionTypeParameterNameValueShadow:{
 type:'boolean',
 {
 allow:[],
 builtinGlobals:false,
 hoist:'functions',
 hoist:'functions-and-types',
 ignoreFunctionTypeParameterNameValueShadow:true,
 ignoreOnInitialization:false,
 ignoreTypeValueShadow:true,
 constinner=getNameRange(variable);
 constouter=getNameRange(scopeVar);

 return!!(
 inner&&
 outer&&
 inner[1]<outer[0]&&
 // Excepts FunctionDeclaration if is {"hoist":"function"}.
 (options.hoist!=='functions'||
 !outerDef||
 outerDef.node.type!==AST_NODE_TYPES.FunctionDeclaration)
 );
 if(!inner||!outer||inner[1]>=outer[0]){
 returnfalse;
 }

 if(!outerDef){
 returntrue;
 }

 if(options.hoist==='functions'){
 return!functionsHoistedNodes.has(outerDef.node.type);
 }

 if(options.hoist==='types'){
 return!typesHoistedNodes.has(outerDef.node.type);
 }

 if(options.hoist==='functions-and-types'){
 return(
 !functionsHoistedNodes.has(outerDef.node.type)&&
 !typesHoistedNodes.has(outerDef.node.type)
 );
 }

 returntrue;
 }

 /**
diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-shadow.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-shadow.shot
Original file line number	Diff line number	Diff line change
Expand Up		@@ -17,18 +17,52 @@ It adds support for TypeScript's `this` parameters and global augmentation, and
		This rule adds the following options:

		```ts
		type AdditionalHoistOptionEntries = 'types' \| 'functions-and-types';

		type HoistOptionEntries =
		\| BaseNoShadowHoistOptionEntries
		\| AdditionalHoistOptionEntries;

		interface Options extends BaseNoShadowOptions {
		hoist?: HoistOptionEntries;
		ignoreTypeValueShadow?: boolean;
		ignoreFunctionTypeParameterNameValueShadow?: boolean;
		}

		const defaultOptions: Options = {
		...baseNoShadowDefaultOptions,
		hoist: 'functions-and-types',
		ignoreTypeValueShadow: true,
		ignoreFunctionTypeParameterNameValueShadow: true,
		};
		```

		### hoist: `types`

		Examples of incorrect code for the `{ "hoist": "types" }` option:

		```ts option='{ "hoist": "types" }' showPlaygroundButton
		type Bar<Foo> = 1;
		type Foo = 1;
		```

		### hoist: `functions-and-types`

		Examples of incorrect code for the `{ "hoist": "functions-and-types" }` option:

		```ts option='{ "hoist": "functions-and-types" }' showPlaygroundButton
		// types
		type Bar<Foo> = 1;
		type Foo = 1;

		// functions
		if (true) {
		let b = 6;
		}

		function b() {}
		```

		### `ignoreTypeValueShadow`

		{/* insert option description */}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -11,7 +11,7 @@
		{
		allow?:string[];
		builtinGlobals?:boolean;
		hoist?:'all'\|'functions'\|'never';
		hoist?:'all'\|'functions'\|'functions-and-types'\|'never'\|'types';
		ignoreFunctionTypeParameterNameValueShadow?:boolean;
		ignoreOnInitialization?:boolean;
		ignoreTypeValueShadow?:boolean;
Expand All		@@ -28,6 +28,13 @@
		AST_NODE_TYPES.TSConstructorType,
		]);

		constfunctionsHoistedNodes=newSet([AST_NODE_TYPES.FunctionDeclaration]);

		consttypesHoistedNodes=newSet([
		AST_NODE_TYPES.TSInterfaceDeclaration,
		AST_NODE_TYPES.TSTypeAliasDeclaration,
		]);

		exportdefaultcreateRule<Options,MessageIds>({
		name:'no-shadow',
		meta:{
Expand DownExpand Up		@@ -63,7 +70,7 @@
		type:'string',
		description:
		'Whether to report shadowing before outer functions or variables are defined.',
		enum:['all','functions','never'],
		enum:['all','functions','functions-and-types','never','types'],
Copy link Member JoshuaKGoldbergJan 7, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I've expanded the existing`hoist` configuration to include`types` and`functions-and-types`. I don't think this is optional, as permutations need to be manually set ... Yeah, I'm thinking this exposes a general learning: don't use`enum` option types for"when do I X?" questions. Eventually folks will want to do something that could be better expressed as an object. I.e. here: {optionName:{functions:true,types:true,variables:true}} ...but, to your point, I think it would be even more disruptive to extend the base rule option to a wholly different format in that way. For now I think the`enum` is fine. If this rule gets even more complicated over time then we can always switch to the object later. tl;dr: agreed as-is 👍 ronami reacted with thumbs up emoji
		},
		ignoreFunctionTypeParameterNameValueShadow:{
		type:'boolean',
Expand All		@@ -88,7 +95,7 @@
		{
		allow:[],
		builtinGlobals:false,
		hoist:'functions',
		hoist:'functions-and-types',
		ignoreFunctionTypeParameterNameValueShadow:true,
		ignoreOnInitialization:false,
		ignoreTypeValueShadow:true,
Expand DownExpand Up		@@ -513,15 +520,30 @@
		constinner=getNameRange(variable);
		constouter=getNameRange(scopeVar);

		return!!(
		inner&&
		outer&&
		inner[1]<outer[0]&&
		// Excepts FunctionDeclaration if is {"hoist":"function"}.
		(options.hoist!=='functions'\|\|
		!outerDef\|\|
		outerDef.node.type!==AST_NODE_TYPES.FunctionDeclaration)
		);
		if(!inner\|\|!outer\|\|inner[1]>=outer[0]){
		returnfalse;
		}

		if(!outerDef){
		returntrue;
Check warning on line 528 in packages/eslint-plugin/src/rules/no-shadow.ts View check run for this annotation Codecov/ codecov/patch packages/eslint-plugin/src/rules/no-shadow.ts#L528 `Added line #L528 was not covered by tests`
		}
Comment on lines +527 to +529 Copy link MemberAuthor ronamiJan 12, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I went over this again and made one small change. I think I've introduced an unnecessary change to the original implementation (returning`false` instead of`true` if`outerDef` is`false`, see`7dd6b06`). I don't think this code is reachable (all tests seem to pass regardless of this change), and I couldn't reproduce this in a test. Still, just to be sure, I've changed this to match the original implementation. JoshuaKGoldberg reacted with thumbs up emoji Copy link MemberAuthor ronamiJan 12, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Now that it's on its own`if` statement, it fails codecov (even though this wasn't covered previously but was in a single long binary operation). I'm a bit unsure what the best way to tackle this is. I'd normally use`nullThrows` or a type assertion, but I'm not entirely sure this really is an unreachable line. This part of the code seems to originate fromthe original eslint rule andis there from the very beginning. Copy link Member JoshuaKGoldbergJan 13, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Yeah I'd just ignore Codecov here. Its reporter isn't perfect. ronami reacted with thumbs up emoji

		if(options.hoist==='functions'){
		return!functionsHoistedNodes.has(outerDef.node.type);
		}

		if(options.hoist==='types'){
		return!typesHoistedNodes.has(outerDef.node.type);
		}

		if(options.hoist==='functions-and-types'){
		return(
		!functionsHoistedNodes.has(outerDef.node.type)&&
		!typesHoistedNodes.has(outerDef.node.type)
		);
		}

		returntrue;
		}

		/**
Expand Down