Skip to content

Commit

Permalink
qualify columns
Browse files Browse the repository at this point in the history
  • Loading branch information
@mgravell
mgravell committed Sep 15, 2023
1 parent e76a10c commit 3d82f26
Show file tree
Hide file tree
Showing 8 changed files with 154 additions and 62 deletions.
14 changes: 8 additions & 6 deletions src/Dapper.AOT.Analyzers/CodeAnalysis/Diagnostics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,19 @@ internal static readonly DiagnosticDescriptor
UpdateWithoutWhere = new("DAP224", "UPDATE without WHERE",
"UPDATE statement lacks WHERE clause (L{0} C{1})", Category.Sql, DiagnosticSeverity.Warning, true),

FromMultiTableMissingAlias = new("DAP225", "UPDATE without WHERE",
FromMultiTableMissingAlias = new("DAP225", "Multi-element FROM missing alias",
"FROM expressions with multiple elements should use aliases (L{0} C{1})", Category.Sql, DiagnosticSeverity.Warning, true),
NonIntegerTop = new("DAP226", "UPDATE without WHERE",
FromMultiTableUnqualifiedColumn = new("DAP226", "Multi-element FROM with unqualified column",
"FROM expressions with multiple elements should qualify all columns; it is unclear where '{0}' is located (L{1} C{2})", Category.Sql, DiagnosticSeverity.Warning, true),
NonIntegerTop = new("DAP227", "Non-integer TOP",
"TOP literals should be integers (L{0} C{1})", Category.Sql, DiagnosticSeverity.Error, true),
NonPositiveTop = new("DAP227", "UPDATE without WHERE",
NonPositiveTop = new("DAP228", "Non-positive TOP",
"TOP literals should be positive (L{0} C{1})", Category.Sql, DiagnosticSeverity.Error, true),
SelectFirstTopError = new("DAP228", "UPDATE without WHERE",
SelectFirstTopError = new("DAP229", "SELECT for First* with invalid TOP",
"SELECT for First* should use TOP 1 (L{0} C{1})", Category.Sql, DiagnosticSeverity.Warning, true),
SelectSingleTopError = new("DAP229", "UPDATE without WHERE",
SelectSingleTopError = new("DAP230", "SELECT for Single* with invalid TOP",
"SELECT for Single* should use TOP 2; if you do not need to test over-read, use First* (L{0} C{1})", Category.Sql, DiagnosticSeverity.Warning, true),
SelectSingleRowWithoutWhere = new("DAP230", "UPDATE without WHERE",
SelectSingleRowWithoutWhere = new("DAP231", "SELECT for single row without WHERE",
"SELECT for single row without WHERE or (TOP and ORDER BY) (L{0} C{1})", Category.Sql, DiagnosticSeverity.Warning, true);


Expand Down
2 changes: 2 additions & 0 deletions src/Dapper.AOT.Analyzers/Internal/DiagnosticTSqlProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ protected override void OnDeleteWithoutWhere(Location location)

protected override void OnFromMultiTableMissingAlias(Location location)
=> AddDiagnostic(Diagnostics.FromMultiTableMissingAlias, location, location.Line, location.Column);
protected override void OnFromMultiTableUnqualifiedColumn(Location location, string name)
=> AddDiagnostic(Diagnostics.FromMultiTableUnqualifiedColumn, location, name, location.Line, location.Column);
protected override void OnNonIntegerTop(Location location)
=> AddDiagnostic(Diagnostics.NonIntegerTop, location, location.Line, location.Column);
protected override void OnNonPositiveTop(Location location)
Expand Down
62 changes: 57 additions & 5 deletions src/Dapper.AOT.Analyzers/SqlAnalysis/TSqlProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ protected virtual void OnNonIntegerTop(Location location)
=> OnError($"TOP literals should be integers", location);
protected virtual void OnFromMultiTableMissingAlias(Location location)
=> OnError($"FROM expressions with multiple elements should use aliases", location);
protected virtual void OnFromMultiTableUnqualifiedColumn(Location location, string name)
=> OnError($"FROM expressions with multiple elements should qualify all columns; it is unclear where '{name}' is located", location);


internal readonly struct Location
Expand Down Expand Up @@ -677,29 +679,79 @@ private bool IsMultiTable(FromClause? from)
return tables.Count > 1 || tables[0] is JoinTableReference;
}

private bool _fromDemandAlias;
public override void ExplicitVisit(SelectStatement node)
{
bool oldDemandAlias = _demandAliases;
try
{
// set ambient state so we can complain more as we walk the nodes
_demandAliases = IsMultiTable((node.QueryExpression as QuerySpecification)?.FromClause);
base.ExplicitVisit(node);
}
finally
{
_demandAliases = oldDemandAlias;
}
}
public override void ExplicitVisit(UpdateStatement node)
{
bool oldDemandAlias = _demandAliases;
try
{
// set ambient state so we can complain more as we walk the nodes
_demandAliases = IsMultiTable(node.UpdateSpecification.FromClause);
base.ExplicitVisit(node);
}
finally
{
_demandAliases = oldDemandAlias;
}
}
public override void ExplicitVisit(DeleteStatement node)
{
bool oldDemandAlias = _demandAliases;
try
{
// set ambient state so we can complain more as we walk the nodes
_demandAliases = IsMultiTable(node.DeleteSpecification.FromClause);
base.ExplicitVisit(node);
}
finally
{
_demandAliases = oldDemandAlias;
}
}
private bool _demandAliases;
public override void ExplicitVisit(FromClause node)
{
bool oldDemandAlias = _fromDemandAlias;
bool oldDemandAlias = _demandAliases;
try
{
// set ambient state so we can complain more as we walk the nodes
_fromDemandAlias = IsMultiTable(node);
_demandAliases = IsMultiTable(node);
base.ExplicitVisit(node);
}
finally
{
_fromDemandAlias = oldDemandAlias;
_demandAliases = oldDemandAlias;
}
}
public override void Visit(TableReferenceWithAlias node)
{
if (_fromDemandAlias && string.IsNullOrWhiteSpace(node.Alias?.Value))
if (_demandAliases && string.IsNullOrWhiteSpace(node.Alias?.Value))
{
parser.OnFromMultiTableMissingAlias(new(node));
}
base.Visit(node);
}
public override void Visit(ColumnReferenceExpression node)
{
if (_demandAliases && node.MultiPartIdentifier.Count == 1)
{
parser.OnFromMultiTableUnqualifiedColumn(new(node), node.MultiPartIdentifier[0].Value);
}
base.Visit(node);
}

public override void Visit(DeleteSpecification node)
{
Expand Down
2 changes: 1 addition & 1 deletion test/Dapper.AOT.Test/Interceptors/QueryDetection.output.netfx.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Dapper.AOT handled 11 of 11 enabled call-sites using 7 interceptors, 1 commands
Warning DAP025 Interceptors/QueryDetection.input.cs L12 C12
The command has a query that will be ignored

Warning DAP230 Interceptors/QueryDetection.input.cs L17 C35
Warning DAP231 Interceptors/QueryDetection.input.cs L17 C35
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP219 Interceptors/QueryDetection.input.cs L17 C42
Expand Down
2 changes: 1 addition & 1 deletion test/Dapper.AOT.Test/Interceptors/QueryDetection.output.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Dapper.AOT handled 11 of 11 enabled call-sites using 7 interceptors, 1 commands
Warning DAP025 Interceptors/QueryDetection.input.cs L12 C12
The command has a query that will be ignored

Warning DAP230 Interceptors/QueryDetection.input.cs L17 C35
Warning DAP231 Interceptors/QueryDetection.input.cs L17 C35
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP219 Interceptors/QueryDetection.input.cs L17 C42
Expand Down
2 changes: 1 addition & 1 deletion test/Dapper.AOT.Test/Interceptors/TsqlTips.input.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void TopChecks(System.Data.SqlClient.SqlConnection connection)
connection.QuerySingle("select top 2 A, B from SomeTable");

// first/single with join; we'll allow join to function as a filter
connection.QueryFirst("select A, B from SomeTable x inner join Foo y on x.Id = y.Id");
connection.QueryFirst("select x.A, B from SomeTable x inner join Foo y on x.Id = y.Id");

// check for aliases
connection.QueryFirst("select A, B from SomeTable x inner join Foo on x.Id = FromSomewhere");
Expand Down
66 changes: 42 additions & 24 deletions test/Dapper.AOT.Test/Interceptors/TsqlTips.output.netfx.txt
View file
Original file line number Diff line number Diff line change
@@ -1,44 +1,56 @@
Generator produced 41 diagnostics:
Generator produced 47 diagnostics:

Hidden DAP000 L1 C1
Dapper.AOT handled 52 of 54 enabled call-sites using 10 interceptors, 3 commands and 1 readers

Error DAP227 Interceptors/TsqlTips.input.cs L12 C34
Error DAP228 Interceptors/TsqlTips.input.cs L12 C34
TOP literals should be positive (L1 C8)

Error DAP226 Interceptors/TsqlTips.input.cs L14 C34
Error DAP227 Interceptors/TsqlTips.input.cs L14 C34
TOP literals should be integers (L1 C8)

Error DAP206 Interceptors/TsqlTips.input.cs L19 C38
Incorrect syntax near -. (#46010 L1 C12)

Warning DAP228 Interceptors/TsqlTips.input.cs L28 C32
Warning DAP229 Interceptors/TsqlTips.input.cs L28 C32
SELECT for First* should use TOP 1 (L1 C1)

Warning DAP229 Interceptors/TsqlTips.input.cs L29 C33
Warning DAP230 Interceptors/TsqlTips.input.cs L29 C33
SELECT for Single* should use TOP 2; if you do not need to test over-read, use First* (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L31 C32
Warning DAP231 Interceptors/TsqlTips.input.cs L31 C32
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L32 C33
Warning DAP231 Interceptors/TsqlTips.input.cs L32 C33
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L34 C32
Warning DAP231 Interceptors/TsqlTips.input.cs L34 C32
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L35 C33
Warning DAP231 Interceptors/TsqlTips.input.cs L35 C33
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L36 C32
Warning DAP231 Interceptors/TsqlTips.input.cs L36 C32
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L37 C33
Warning DAP231 Interceptors/TsqlTips.input.cs L37 C33
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP226 Interceptors/TsqlTips.input.cs L40 C44
FROM expressions with multiple elements should qualify all columns; it is unclear where 'B' is located (L1 C13)

Warning DAP226 Interceptors/TsqlTips.input.cs L43 C39
FROM expressions with multiple elements should qualify all columns; it is unclear where 'A' is located (L1 C8)

Warning DAP226 Interceptors/TsqlTips.input.cs L43 C42
FROM expressions with multiple elements should qualify all columns; it is unclear where 'B' is located (L1 C11)

Warning DAP225 Interceptors/TsqlTips.input.cs L43 C72
FROM expressions with multiple elements should use aliases (L1 C41)

Warning DAP226 Interceptors/TsqlTips.input.cs L43 C86
FROM expressions with multiple elements should qualify all columns; it is unclear where 'FromSomewhere' is located (L1 C55)

Warning DAP216 Interceptors/TsqlTips.input.cs L53 C29
INSERT should explicitly specify target columns (L1 C1)

Expand All @@ -48,19 +60,19 @@ The INSERT values do not match the target columns (L1 C28)
Error DAP218 Interceptors/TsqlTips.input.cs L59 C53
The INSERT rows have different widths (L1 C25)

Warning DAP230 Interceptors/TsqlTips.input.cs L64 C37
Warning DAP231 Interceptors/TsqlTips.input.cs L64 C37
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP219 Interceptors/TsqlTips.input.cs L64 C44
SELECT columns should be specified explicitly (L1 C8)

Warning DAP230 Interceptors/TsqlTips.input.cs L67 C37
Warning DAP231 Interceptors/TsqlTips.input.cs L67 C37
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP219 Interceptors/TsqlTips.input.cs L67 C44
SELECT columns should be specified explicitly (L1 C8)

Warning DAP230 Interceptors/TsqlTips.input.cs L70 C37
Warning DAP231 Interceptors/TsqlTips.input.cs L70 C37
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP220 Interceptors/TsqlTips.input.cs L79 C38
Expand All @@ -72,19 +84,19 @@ SELECT column name is duplicated: 'Balance' (L1 C17)
Warning DAP222 Interceptors/TsqlTips.input.cs L98 C13
SELECT statement assigns variable and performs reads (L2 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L103 C46
Warning DAP231 Interceptors/TsqlTips.input.cs L103 C46
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP220 Interceptors/TsqlTips.input.cs L103 C53
SELECT column name is missing: 0 (L1 C8)

Warning DAP230 Interceptors/TsqlTips.input.cs L105 C41
Warning DAP231 Interceptors/TsqlTips.input.cs L105 C41
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Info DAP013 Interceptors/TsqlTips.input.cs L111 C24
Tuple-type results are not currently supported

Warning DAP230 Interceptors/TsqlTips.input.cs L111 C52
Warning DAP231 Interceptors/TsqlTips.input.cs L111 C52
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP220 Interceptors/TsqlTips.input.cs L111 C59
Expand All @@ -93,32 +105,38 @@ SELECT column name is missing: 0 (L1 C8)
Info DAP013 Interceptors/TsqlTips.input.cs L117 C24
Tuple-type results are not currently supported

Warning DAP230 Interceptors/TsqlTips.input.cs L117 C48
Warning DAP231 Interceptors/TsqlTips.input.cs L117 C48
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP025 Interceptors/TsqlTips.input.cs L123 C20
The command has a query that will be ignored

Warning DAP225 Interceptors/TsqlTips.input.cs L149 C36
FROM expressions with multiple elements should use aliases (L1 C8)

Warning DAP223 Interceptors/TsqlTips.input.cs L152 C29
DELETE statement lacks WHERE clause (L1 C1)

Warning DAP224 Interceptors/TsqlTips.input.cs L153 C29
UPDATE statement lacks WHERE clause (L1 C1)

Warning DAP229 Interceptors/TsqlTips.input.cs L158 C43
SELECT for Single* should use TOP 2; if you do not need to test over-read, use First* (L1 C1)
Warning DAP225 Interceptors/TsqlTips.input.cs L155 C36
FROM expressions with multiple elements should use aliases (L1 C8)

Warning DAP230 Interceptors/TsqlTips.input.cs L158 C43
SELECT for Single* should use TOP 2; if you do not need to test over-read, use First* (L1 C1)

Warning DAP231 Interceptors/TsqlTips.input.cs L158 C43
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L159 C43
Warning DAP231 Interceptors/TsqlTips.input.cs L159 C43
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP229 Interceptors/TsqlTips.input.cs L161 C43
Warning DAP230 Interceptors/TsqlTips.input.cs L161 C43
SELECT for Single* should use TOP 2; if you do not need to test over-read, use First* (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L161 C43
Warning DAP231 Interceptors/TsqlTips.input.cs L161 C43
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)

Warning DAP230 Interceptors/TsqlTips.input.cs L163 C43
Warning DAP231 Interceptors/TsqlTips.input.cs L163 C43
SELECT for single row without WHERE or (TOP and ORDER BY) (L1 C1)
Loading

0 comments on commit 3d82f26

Please sign in to comment.