-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
C#: Add common sources for Blazor components #18322
base: main
Are you sure you want to change the base?
C#: Add common sources for Blazor components #18322
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
Files not reviewed (2)
- csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected: Language not supported
- csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected: Language not supported
Comments suppressed due to low confidence (2)
csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml:8
- The class name 'NagivationManager' appears to be misspelled. It should be 'NavigationManager'.
- ["Microsoft.AspNetCore.Components", "NagivationManager", True, "get_BaseUri", "", "", "ReturnValue", "remote", "manual"]
csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml:9
- The class name 'NagivationManager' appears to be misspelled. It should be 'NavigationManager'.
- ["Microsoft.AspNetCore.Components", "NagivationManager", True, "get_Uri", "", "", "ReturnValue", "remote", "manual"]
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
Click to show differences in coveragecsharpGenerated file changes for csharp
- Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.Mvc``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",59,2074,152,4
+ Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.Mvc``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",61,2074,152,4
- Totals,,106,12900,400,9
+ Totals,,108,12900,400,9
- Microsoft.AspNetCore.Components,2,2,2,,,,,,,2,,,,,,,,,2,,,1,1
+ Microsoft.AspNetCore.Components,2,4,2,,,,,,,2,,,,,,,,,4,,,1,1 |
csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
Outdated
Show resolved
Hide resolved
csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected
Outdated
Show resolved
Hide resolved
33a3ffe
to
3a15660
Compare
The merge-base changed after approval.
The attributes - `[Parameter]` - `[SupplyParameterFromFormAttribute]` - `[SupplyParameterFromQueryAttribute]` Tell Blazor to initialize the variables with parameters defined by the route/form values/query parameters/etc. Values derived from the URI or form should be classified as `remote` flow sources.
3a15660
to
d0c9ba1
Compare
@@ -5,6 +5,9 @@ extensions: | |||
data: | |||
- ["Microsoft.AspNetCore.Components", "NavigationManager", True, "get_BaseUri", "", "", "ReturnValue", "remote", "manual"] | |||
- ["Microsoft.AspNetCore.Components", "NavigationManager", True, "get_Uri", "", "", "ReturnValue", "remote", "manual"] | |||
- ["Microsoft.AspNetCore.Components", "ParameterAttribute", False, "", "", "Attribute.Getter", "ReturnValue", "remote", "manual"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can properties with [Parameter]
attribute be controlled by the user? Aren't these only component parameters that are required for passing data from component to component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I double checked:
Can properties with [Parameter] attribute be controlled by the user?
Yes, routing parameters for page components which are potentially controlled by a user navigating to those pages.
Example:
@page "/people-search/{year:int}/{name}"
@code {
[Parameter]
public string Name { get; set; }
[Parameter]
public int Year { get; set; }
}
Aren't these only component parameters that are required for passing data from component to component?
So while they aren't "only ... for passing data from component to component", that is the primary use case for them.
Because of that I have decided to remove [Parameter]
from this PR and I will add some additional logic in QL to reason about it better.
csharp/ql/lib/change-notes/2024-12-18-blazor-attribute-sources.md
Outdated
Show resolved
Hide resolved
@tamasvajk I double checked your point about The other two are specialized enough to where they will try to find the parameter (e.g. query strings/form values) that they should be considered sources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The attributes
-[Parameter]
[SupplyParameterFromFormAttribute]
[SupplyParameterFromQueryAttribute]
Tell Blazor to initialize the variables with parameters defined by the route/form values/query parameters/etc. Values derived from the URI or form should be classified as
remote
flow sources.Pull Request checklist
All query authors
- [ ] All new queries have appropriate.qhelp
. See the documentation in this repository.- [ ] New and changed queries have correct query metadata. See the documentation in this repository.Internal query authors only
- [ ] Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to.ql
,.qll
, or.qhelp
files. See the documentation (internal access required).- [ ] Adding a new query? Consider also adding the query to autofix.