Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal] Add StringSyntaxAttributes to SqlJson #3107

Open
edwardneal opened this issue Jan 12, 2025 · 2 comments
Open

[API Proposal] Add StringSyntaxAttributes to SqlJson #3107

edwardneal opened this issue Jan 12, 2025 · 2 comments
Assignees
Labels
💡 Enhancement Issues that are feature requests for the drivers we maintain. ✔️ Triage Done Issues that are triaged by dev team and are in investigation.

Comments

@edwardneal
Copy link
Contributor

Is your feature request related to a problem? Please describe.

.NET 7.0 introduced the StringSyntaxAttribute type, which allows fields, parameters and properties of type string to be marked as containing a specific kind of data. When a member has this attribute applied, data validation rules can be run by Roslyn and Visual Studio to perform static code analysis.

This hasn't yet been threaded through to the new SqlJson type.

Describe the solution you'd like

I'm proposing the API surface change below:

public class SqlJson : INullable
{
-   public SqlJson(string? jsonString) { }
+   public SqlJson([StringSyntax(StringSyntaxAttribute.Json)] string? jsonString) { }

+   [StringSyntax(StringSyntaxAttribute.Json)]
    public string Value { get { } }
}

After this change, a developer who instantiates a SqlJson instance with a hardcoded string will encounter a new warning in Visual Studio and Rider if that string isn't valid JSON: JSON001. These strings will also have their syntax highlighted.

The StringSyntaxAttribute class isn't available in .NET Framework, but the functionality will still work if an internal type with the same name and definition is part of the assembly. NUnit and part of the dotnet arcade repo do this.

Describe alternatives you've considered

This could be made .NET-only if we'd prefer not to duplicate the type definition.

Additional context

I skimmed the ref projects for SqlClient and didn't see any other places where we can add these attributes.

@edwardneal edwardneal added 💡 Enhancement Issues that are feature requests for the drivers we maintain. 🆕 Triage Needed For new issues, not triaged yet. labels Jan 12, 2025
@edwardneal edwardneal changed the title [API Request] Add StringSyntaxAttributes to SqlJson [API Proposal] Add StringSyntaxAttributes to SqlJson Jan 13, 2025
@mdaigle
Copy link
Contributor

mdaigle commented Jan 14, 2025

@apoorvdeshmukh would like to get your thoughts on this. What validation do we currently have that user provided JSON is syntactically valid? Do we have an API that accepts JSON documents directly?

@mdaigle mdaigle added ✔️ Triage Done Issues that are triaged by dev team and are in investigation. and removed 🆕 Triage Needed For new issues, not triaged yet. labels Jan 14, 2025
@edwardneal
Copy link
Contributor Author

To chime in: we call SqlJson.ValidateJson to validate that the string passed to the constructor is valid at compile time, and there's a constructor which accepts a JsonDocument directly. Adding StringSyntaxAttribute to these two members simply enables static code analysis and syntax highlighting within Visual Studio/Rider, it doesn't have a runtime impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Enhancement Issues that are feature requests for the drivers we maintain. ✔️ Triage Done Issues that are triaged by dev team and are in investigation.
Projects
None yet
Development

No branches or pull requests

3 participants