Quality: Potential NullReferenceException in BaseSettings Constructor#1212
Conversation
The constructor of `BaseSettings<T>` uses `Activator.CreateInstance(typeof(T))` to initialize the `Component` property. If `T` does not have a parameterless constructor or if `T` is an interface or abstract class, this will throw an exception at runtime, potentially causing a crash. Affected files: BaseSettings.cs Signed-off-by: BachDEV <1437214+bachdev@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens BaseSettings<T> initialization by eliminating runtime Activator.CreateInstance failures and moving instantiation requirements to compile time.
Changes:
- Add
where T : new()constraint toBaseSettings<T>to require a public parameterless constructor. - Replace
Activator.CreateInstance(typeof(T))withnew T()in the constructor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| namespace AutoDarkModeLib.ComponentSettings; | ||
|
|
||
| public class BaseSettings<T> : ISwitchComponentSettings<T> | ||
| public class BaseSettings<T> : ISwitchComponentSettings<T> where T : new() |
There was a problem hiding this comment.
BaseSettings<T> now requires where T : new(), but derived generic types must also guarantee that constraint. BaseSettingsEnabled<T> : BaseSettings<T> currently has no new() constraint, which will cause a compile-time error. Update BaseSettingsEnabled<T> (and any other subclasses) to include where T : new() or otherwise satisfy the base constraint.
| namespace AutoDarkModeLib.ComponentSettings; | ||
|
|
||
| public class BaseSettings<T> : ISwitchComponentSettings<T> | ||
| public class BaseSettings<T> : ISwitchComponentSettings<T> where T : new() |
There was a problem hiding this comment.
Adding where T : new() to this public generic type is a source/binary breaking change for any external callers that previously used BaseSettings<T> with types lacking a public parameterless constructor (or abstract/interface types). If avoiding a breaking change is important, consider keeping the unconstrained type parameter and handling instantiation failures explicitly (e.g., custom exception/message) instead of enforcing a new() constraint.
|
@copilot apply changes based on the comments in this thread |
…erenceexception-in-base
Problem
The constructor of
BaseSettings<T>usesActivator.CreateInstance(typeof(T))to initialize theComponentproperty. IfTdoes not have a parameterless constructor or ifTis an interface or abstract class, this will throw an exception at runtime, potentially causing a crash.Severity:
highFile:
AutoDarkModeLib/ComponentSettings/BaseSettings.csSolution
Add a constraint to ensure
Thas a parameterless constructor by usingwhere T : new()in the class definition. This will enforce compile-time checks and prevent runtime exceptions.Changes
AutoDarkModeLib/ComponentSettings/BaseSettings.cs(modified)Testing