Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Several manual and stress tests retain obsolete reflected type and stack-frame names and will fail or stop filtering correctly.
Pull request overview
Refactors internal TDS parser types into focused Parser, Parser.Login, and Parser.Tokens namespaces without intended public API changes.
Changes:
- Splits parser helpers, metadata, login, token, and native-handle types into dedicated files.
- Updates production and test imports for the new namespaces.
- Relocates existing public enums into standalone files.
File summaries
| File | Description |
|---|---|
tests/UnitTests/TdsParserInternalsTest.cs |
Imports parser namespace. |
tests/UnitTests/SimulatedServerTests/TdsTokenBoundsTests.cs |
Imports parser namespace. |
tests/UnitTests/SimulatedServerTests/FeatureExtAckBoundsTests.cs |
Imports parser namespace. |
tests/UnitTests/Microsoft/Data/SqlClient/TdsParserStateObjectNativeTests.cs |
Imports parser namespace. |
tests/UnitTests/Microsoft/Data/SqlClient/StateSnapshotPacketRecyclingTests.cs |
Imports parser namespace. |
tests/UnitTests/Microsoft/Data/SqlClient/SqlMetaDataSetTest.cs |
Imports token namespace. |
tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniPacketTests.cs |
Imports parser namespace. |
tests/UnitTests/Microsoft/Data/Common/AdapterUtilTest.cs |
Imports parser namespace. |
tests/FunctionalTests/TdsParserStateObject.TestHarness.cs |
Moves harness into parser namespace. |
tests/FunctionalTests/MultiplexerTests.cs |
Imports parser namespace. |
tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj |
Updates relocated source paths. |
src/Microsoft/Data/SqlTypes/SqlVector.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/TransparentNetworkResolutionState.cs |
Extracts TNIR state enum. |
src/Microsoft/Data/SqlClient/TransactionRequest.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs |
Removes split-out helper definitions. |
src/Microsoft/Data/SqlClient/TdsParameterSetter.cs |
Removes relocated setter. |
src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SSPI/NativeSspiContextProvider.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlUtil.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlTransaction.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlParameter.cs |
Imports parser and token namespaces. |
src/Microsoft/Data/SqlClient/SqlMetaDataFactory.DataTypes.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlEnvChange.cs |
Imports token namespace. |
src/Microsoft/Data/SqlClient/SqlEnums.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlDbColumn.cs |
Imports parser metadata types. |
src/Microsoft/Data/SqlClient/SqlDataReader.cs |
Imports parser metadata types. |
src/Microsoft/Data/SqlClient/SqlCredential.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlConnectionStringBuilder.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlConnectionOverrides.cs |
Extracts public enum. |
src/Microsoft/Data/SqlClient/SqlConnectionOptions.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlConnectionIPAddressPreference.cs |
Extracts public enum. |
src/Microsoft/Data/SqlClient/SqlConnectionColumnEncryptionSetting.cs |
Extracts public enum. |
src/Microsoft/Data/SqlClient/SqlConnectionAttestationProtocol.cs |
Extracts public enum. |
src/Microsoft/Data/SqlClient/SqlConnection.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/SqlCommandColumnEncryptionSetting.cs |
Extracts public enum. |
src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlCommand.cs |
Imports parser and token namespaces. |
src/Microsoft/Data/SqlClient/SqlCommand.Batch.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlCollation.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs |
Imports parser metadata types. |
src/Microsoft/Data/SqlClient/SqlBulkCopy.cs |
Imports parser metadata types. |
src/Microsoft/Data/SqlClient/SqlBuffer.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/Server/ValueUtilsSmi.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/Server/MetadataUtilsSmi.cs |
Imports parser namespace. |
src/Microsoft/Data/SqlClient/Parser/Tokens/SqlReturnValue.cs |
Extracts return-value metadata. |
src/Microsoft/Data/SqlClient/Parser/Tokens/SqlMetaDataXmlSchemaCollection.cs |
Extracts XML metadata. |
src/Microsoft/Data/SqlClient/Parser/Tokens/SqlMetaDataUdt.cs |
Extracts UDT metadata. |
src/Microsoft/Data/SqlClient/Parser/Tokens/SqlMetaDataPriv.cs |
Consolidates private metadata. |
src/Microsoft/Data/SqlClient/Parser/Tokens/SqlFedAuthInfo.cs |
Extracts federated-auth token data. |
src/Microsoft/Data/SqlClient/Parser/Tokens/RoutingInfo.cs |
Extracts routing token data. |
src/Microsoft/Data/SqlClient/Parser/Tokens/MultiPartTableName.cs |
Extracts multipart-name parsing. |
src/Microsoft/Data/SqlClient/Parser/Tokens/_SqlMetaDataSetCollection.cs |
Extracts metadata collection. |
src/Microsoft/Data/SqlClient/Parser/Tokens/_SqlMetaDataSet.cs |
Consolidates metadata-set logic. |
src/Microsoft/Data/SqlClient/Parser/Tokens/_SqlMetaData.cs |
Extracts column metadata. |
src/Microsoft/Data/SqlClient/Parser/TdsValueSetter.cs |
Moves value writer into parser namespace. |
src/Microsoft/Data/SqlClient/Parser/TdsRecordBufferSetter.cs |
Moves record writer into parser namespace. |
src/Microsoft/Data/SqlClient/Parser/TdsParserStaticMethods.cs |
Moves parser utilities. |
src/Microsoft/Data/SqlClient/Parser/TdsParserStateObjectNative.cs |
Moves native parser state. |
src/Microsoft/Data/SqlClient/Parser/TdsParserStateObjectManaged.netcore.cs |
Moves managed parser state. |
src/Microsoft/Data/SqlClient/Parser/TdsParserStateObjectFactory.cs |
Moves parser-state factory. |
src/Microsoft/Data/SqlClient/Parser/TdsParserStateObject.Multiplexer.cs |
Moves multiplexer implementation. |
src/Microsoft/Data/SqlClient/Parser/TdsParserStateObject.cs |
Moves core parser state. |
src/Microsoft/Data/SqlClient/Parser/TdsParserState.cs |
Extracts parser-state enum. |
src/Microsoft/Data/SqlClient/Parser/TdsParserSessionPool.cs |
Moves session pool. |
src/Microsoft/Data/SqlClient/Parser/TdsParser.cs |
Moves core parser. |
src/Microsoft/Data/SqlClient/Parser/TdsParameterSetter.cs |
Relocates parameter setter. |
src/Microsoft/Data/SqlClient/Parser/TdsOperationsStatus.cs |
Extracts operation-status enum. |
src/Microsoft/Data/SqlClient/Parser/SslProtocolHelper.cs |
Extracts TLS warning helper. |
src/Microsoft/Data/SqlClient/Parser/SNIPacket.cs |
Extracts native packet handle. |
src/Microsoft/Data/SqlClient/Parser/SNILoadHandle.cs |
Moves native SNI loader. |
src/Microsoft/Data/SqlClient/Parser/SNIHandle.cs |
Extracts native connection handle. |
src/Microsoft/Data/SqlClient/Parser/SniContext.cs |
Extracts SNI context enum. |
src/Microsoft/Data/SqlClient/Parser/SessionHandle.netfx.cs |
Moves .NET Framework session handle. |
src/Microsoft/Data/SqlClient/Parser/SessionHandle.netcore.cs |
Moves .NET session handle. |
src/Microsoft/Data/SqlClient/Parser/RunBehavior.cs |
Extracts parser run behavior. |
src/Microsoft/Data/SqlClient/Parser/ParsingErrorState.cs |
Extracts parsing-error enum. |
src/Microsoft/Data/SqlClient/Parser/PacketHandle.netfx.cs |
Moves .NET Framework packet handle. |
src/Microsoft/Data/SqlClient/Parser/PacketHandle.netcore.cs |
Moves .NET packet handle. |
src/Microsoft/Data/SqlClient/Parser/Packet.cs |
Moves packet reconstruction helper. |
src/Microsoft/Data/SqlClient/Parser/Login/SqlLogin.cs |
Extracts login state. |
src/Microsoft/Data/SqlClient/Parser/Login/SqlFedAuthToken.cs |
Extracts federated-auth token. |
src/Microsoft/Data/SqlClient/Parser/Login/PreLoginOptions.cs |
Extracts pre-login options. |
src/Microsoft/Data/SqlClient/Parser/Login/PreLoginHandshakeStatus.cs |
Extracts handshake status. |
src/Microsoft/Data/SqlClient/Parser/Login/FederatedAuthenticationFeatureExtensionData.cs |
Extracts federated-auth extension data. |
src/Microsoft/Data/SqlClient/Parser/Login/EncryptionOptions.cs |
Extracts encryption options. |
src/Microsoft/Data/SqlClient/Parser/LastIOTimer.cs |
Extracts I/O timer holder. |
src/Microsoft/Data/SqlClient/Parser/DescribeParameterEncryptionResultSet3.cs |
Extracts third result-set ordinals. |
src/Microsoft/Data/SqlClient/Parser/DescribeParameterEncryptionResultSet2.cs |
Extracts second result-set ordinals. |
src/Microsoft/Data/SqlClient/Parser/DescribeParameterEncryptionResultSet1.cs |
Extracts first result-set ordinals. |
src/Microsoft/Data/SqlClient/Parser/CallbackType.cs |
Extracts callback enum. |
src/Microsoft/Data/SqlClient/Parser/_SqlRPC.cs |
Extracts RPC state. |
src/Microsoft/Data/SqlClient/ManagedSni/SslOverTdsStream.netcore.cs |
Imports parser constants. |
src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniPacket.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniNpHandle.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniMarsHandle.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniMarsConnection.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniLoadHandle.netcore.cs |
Imports parser/login types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniHandle.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniError.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/SniCommon.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/ManagedSni/LocalDB.netcore.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/LocalDb/LocalDbApi.cs |
Imports parser types. |
src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs |
Imports parser, login, and token types. |
src/Microsoft/Data/SqlClient/Connection/ServerInfo.cs |
Imports parser routing types. |
src/Microsoft/Data/SqlClient/Connection/ConnectionCapabilities.cs |
Imports parser constants. |
src/Microsoft/Data/SqlClient/AlwaysEncryptedHelperClasses.cs |
Removes relocated metadata partials. |
src/Microsoft/Data/SqlClient/ActiveDirectoryAuthenticationTimeoutRetryHelper.cs |
Imports login token type. |
src/Microsoft/Data/Sql/SqlDataSourceEnumeratorNativeHelper.cs |
Imports parser types. |
src/Microsoft/Data/Common/AdapterUtil.cs |
Imports parser types. |
src/Interop/Windows/Sni/SniNativeWrapper.cs |
Imports relocated native handles. |
src/Interop/Windows/Sni/SniNativeMethodsX86.netfx.cs |
Imports parser handles. |
src/Interop/Windows/Sni/SniNativeMethodsX64.netfx.cs |
Imports parser handles. |
src/Interop/Windows/Sni/SniNativeMethodsNotSupported.netfx.cs |
Imports parser handles. |
src/Interop/Windows/Sni/SniNativeMethodsArm64.netfx.cs |
Imports parser handles. |
src/Interop/Windows/Sni/SniNativeMethods.netcore.cs |
Imports parser handles. |
src/Interop/Windows/Sni/ISniNativeMethods.cs |
Imports parser handles. |
Review details
Suppressed comments (3)
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Parser/TdsParser.cs:40
- This namespace move leaves reflection consumers pointing at
Microsoft.Data.SqlClient.TdsParser:ConnectionHelper.cs:52andTdsParserHelper.cs:13now receivenullfromAssembly.GetType(the former dereferences it during static initialization), and the stress-test filter atFilteredDefaultTraceListener.cs:84no longer matches the new stack-frame name. Update those strings toMicrosoft.Data.SqlClient.Parser.TdsParseras part of this move.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Parser/TdsParserStateObject.cs:28 - Several reflection-based tests still request
Microsoft.Data.SqlClient.TdsParserStateObject(ConnectionHelper.cs:57,TdsParserStateObjectHelper.cs:27, andMARSTest.cs:87/135). After this move,Assembly.GetTypereturnsnull, causing static-initializer assertion/null-reference failures; the stress filter atFilteredDefaultTraceListener.cs:85-86also stops matching. Update all of these strings to the newMicrosoft.Data.SqlClient.Parser.TdsParserStateObjectname.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Parser/TdsParserStateObjectManaged.netcore.cs:21 TdsParserStateObjectHelper.cs:48still reflectsMicrosoft.Data.SqlClient.ManagedSni.TdsParserStateObjectManaged. On .NET builds this now resolves tonull; the helper treats that as framework absence and later throws when a managed session handle is requested. Update the reflected name toMicrosoft.Data.SqlClient.Parser.TdsParserStateObjectManaged.
- Files reviewed: 126/126 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
This should be named SslProtocolsHelper.cs to match the containing type name.
There was a problem hiding this comment.
This should be named TdsOperationStatus.cs to match the containing type name.
There was a problem hiding this comment.
This, SNIPacket and SNIHandle are native SNI types, it should go in a new NativeSni namespace (to align with the existing ManagedSni one)
There was a problem hiding this comment.
I like your thinking. This makes loads of sense to me, so despite it growing the scope of the changes in this PR, it's going in.
| public const byte FEATUREEXT_USERAGENT = 0x10; | ||
|
|
||
| [Flags] | ||
| public enum FeatureExtension : uint |
There was a problem hiding this comment.
What's the rationale behind leaving these three enums in situ?
There was a problem hiding this comment.
Basically ... TdsEnums was too much for me to look into 😅
Most of the file isn't actually enums, they're constants. Some of them should be enums, some of them are better off as constants. It seemed like a bigger change than I wanted to tackle with these mostly unoffensive changes.
Consolidate partial classes Add references to Parser namespace
Add references to Parser namespace
Add references where necessary
Move is made wholesale to preserve history
Moved wholesale to retain file history
Move TdsParserStateObjectManaged to Parser namespace
d3821a1 to
2bbf4b1
Compare
This creates a stub namespace that native SNI will go into
There was a problem hiding this comment.
🟡 Changes recommended
Reflection-based manual and stress tests still use the former parser type names, causing failures after the namespace migration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Parser/TdsParser.cs:40
- The namespace move was not propagated to reflection-based test infrastructure.
tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionHelper.cs:52andTdsParserHelper.cs:13still requestMicrosoft.Data.SqlClient.TdsParser;Assembly.GetTypenow returns null and their static field initialization dereferences it. Update those type-name strings toMicrosoft.Data.SqlClient.Parser.TdsParser; the stress-test frame filter atFilteredDefaultTraceListener.cs:84also needs the new prefix.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Parser/TdsParserStateObject.cs:28 - Several manual tests still resolve
Microsoft.Data.SqlClient.TdsParserStateObjectby name (MARSTest.cs:87,135,ConnectionHelper.cs:57, andTdsParserStateObjectHelper.cs:27). After this move those lookups return null, and the subsequentGetFieldcalls or assertions fail. Update all reflection strings and the stress-test stack-frame entries toMicrosoft.Data.SqlClient.Parser.TdsParserStateObject.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Parser/TdsParserStateObjectManaged.netcore.cs:21 TdsParserStateObjectHelper.cs:48still looks up this type asMicrosoft.Data.SqlClient.ManagedSni.TdsParserStateObjectManaged. The lookup now returns null, soGetSessionHandlethrows “Library being tested does not implement TdsParserStateObjectManaged” for the managed implementation. Update the reflection string toMicrosoft.Data.SqlClient.Parser.TdsParserStateObjectManaged.
- Files reviewed: 126/126 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
| using System; | ||
| using System.Data; | ||
|
|
||
| namespace Microsoft.Data.SqlClient.Parser.Tokens; |
Description
Refactors the TDS parser implementation into focused files and namespaces without changing its public API or intended behavior.
Microsoft.Data.SqlClient.Parser.Parser.LoginandParser.Tokens.This is layer 1 of 3 in the TDS parser organization stack and targets
main.Issues
No linked issue; this is an internal code-organization refactor.
Testing
No behavior change is intended. Existing functional and unit test sources were updated to use the reorganized parser namespaces; CI will exercise the supported target frameworks and platforms.
Guidelines