Skip to content

Commit

Permalink
Merge "stdlib: Add type checks" into main
Browse files Browse the repository at this point in the history
  • Loading branch information
Treehugger Robot authored and Gerrit Code Review committed Nov 21, 2024
2 parents 65d6491 + 88be776 commit 834aa3b
Show file tree
Hide file tree
Showing 127 changed files with 1,471 additions and 1,443 deletions.
53 changes: 39 additions & 14 deletions python/generators/sql_processing/docs_parse.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from python.generators.sql_processing.docs_extractor import DocsExtractor
from python.generators.sql_processing.utils import ObjKind
from python.generators.sql_processing.utils import COLUMN_TYPES, MACRO_ARG_TYPES

from python.generators.sql_processing.utils import ALLOWED_PREFIXES
from python.generators.sql_processing.utils import OBJECT_NAME_ALLOWLIST
Expand All @@ -38,7 +39,9 @@ def _is_snake_case(s: str) -> bool:


def parse_comment(comment: str) -> str:
"""Parse a SQL comment (i.e. -- Foo\n -- bar.) into a string (i.e. "Foo bar.")."""
"""
Parse a SQL comment (i.e. -- Foo\n -- bar.) into a string (i.e. "Foo bar.").
"""
return ' '.join(line.strip().lstrip('--').lstrip()
for line in comment.strip().split('\n'))

Expand Down Expand Up @@ -68,9 +71,7 @@ def get_module_prefix_error(name: str, path: str, module: str) -> Optional[str]:


class Arg(NamedTuple):
# TODO(b/307926059): the type is missing on old-style documentation for
# tables. Make it "str" after stdlib is migrated.
type: Optional[str]
type: str
description: str


Expand Down Expand Up @@ -100,23 +101,47 @@ def _parse_desc_not_empty(self, desc: str):
self._error('Description of the table/view/function/macro is missing')
return desc.strip()

def _parse_columns(self, schema: str) -> Dict[str, Arg]:
def _parse_columns(self, schema: str, kind: ObjKind) -> Dict[str, Arg]:
columns = self._parse_args_definition(schema) if schema else {}
for column in columns:
if not columns[column].description:
self._error(f'Column "{column}" is missing a description. Please add a '
for column_name, properties in columns.items():
if not properties.description:
self._error(f'Column "{column_name}" is missing a description. Please add a '
'comment in front of the column definition')
continue

upper_arg_type = properties.type.upper()
if kind is ObjKind.table_function:
if upper_arg_type not in COLUMN_TYPES:
self._error(
f'Table function column "{column_name}" has unsupported type "{properties.type}".')
elif kind is ObjKind.table_view:
if upper_arg_type not in COLUMN_TYPES:
self._error(
f'Table/view column "{column_name}" has unsupported type "{properties.type}".')
else:
self._error(f'This Perfetto SQL object doesnt support columns".')

return columns

def _parse_args(self, sql_args_str: str) -> Dict[str, Arg]:
def _parse_args(self, sql_args_str: str, kind: ObjKind) -> Dict[str, Arg]:
args = self._parse_args_definition(sql_args_str)

for arg in args:
if not args[arg].description:
self._error(f'Arg "{arg}" is missing a description. '
'Please add a comment in front of the arg definition.')
upper_arg_type = args[arg].type.upper()
if (kind is ObjKind.function or kind is ObjKind.table_function):
if upper_arg_type not in COLUMN_TYPES:
self._error(
f'Function arg "{arg}" has unsupported type "{args[arg].type}".')
elif (kind is ObjKind.macro):
if upper_arg_type not in MACRO_ARG_TYPES:
self._error(
f'Macro arg "{arg}" has unsupported type "{args[arg].type}".')
else:
self._error(f'This Perfetto SQL object doesnt support types".')

return args

# Parse function argument definition list or a table schema, e.g.
Expand Down Expand Up @@ -202,7 +227,7 @@ def parse(self, doc: DocsExtractor.Extract) -> Optional[TableOrView]:
name=self._parse_name(),
type=type,
desc=self._parse_desc_not_empty(doc.description),
cols=self._parse_columns(schema),
cols=self._parse_columns(schema, ObjKind.table_view),
)


Expand Down Expand Up @@ -253,7 +278,7 @@ def parse(self, doc: DocsExtractor.Extract) -> Optional[Function]:
return Function(
name=name,
desc=self._parse_desc_not_empty(doc.description),
args=self._parse_args(args),
args=self._parse_args(args, ObjKind.function),
return_type=ret_type,
return_desc=ret_desc,
)
Expand Down Expand Up @@ -301,8 +326,8 @@ def parse(self, doc: DocsExtractor.Extract) -> Optional[TableFunction]:
return TableFunction(
name=name,
desc=self._parse_desc_not_empty(doc.description),
cols=self._parse_columns(columns),
args=self._parse_args(args),
cols=self._parse_columns(columns, ObjKind.table_function),
args=self._parse_args(args, ObjKind.table_function),
)


Expand Down Expand Up @@ -352,7 +377,7 @@ def parse(self, doc: DocsExtractor.Extract) -> Optional[Macro]:
desc=self._parse_desc_not_empty(doc.description),
return_desc=parse_comment(return_desc),
return_type=return_type,
args=self._parse_args(args),
args=self._parse_args(args, ObjKind.macro),
)


Expand Down
36 changes: 20 additions & 16 deletions python/generators/sql_processing/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,26 @@
import os
from typing import Dict, List

ALLOWED_PREFIXES = {
'android': ['heap_graph', 'memory'],
'counters': ['counter'],
'chrome/util': ['cr'],
'intervals': ['interval'],
'graphs': ['graph'],
'slices': ['slice', 'thread_slice', 'process_slice'],
'linux': ['cpu', 'memory'],
'stacks': ['cpu_profiling'],
}

# Allows for nonstandard object names.
OBJECT_NAME_ALLOWLIST = {
'graphs/partition.sql': ['tree_structural_partition_by_group'],
}

COLUMN_TYPES = ['LONG', 'DOUBLE', 'STRING', 'BOOL', 'BYTES']

MACRO_ARG_TYPES = ['TABLEORSUBQUERY', 'EXPR', 'COLUMNNAME']

NAME = r'[a-zA-Z_\d\{\}]+'
ANY_WORDS = r'[^\s].*'
ANY_NON_QUOTE = r'[^\']*.*'
Expand Down Expand Up @@ -107,22 +127,6 @@ class ObjKind(str, Enum):
ObjKind.include: INCLUDE_PATTERN
}

ALLOWED_PREFIXES = {
'android': ['heap_graph', 'memory'],
'counters': ['counter'],
'chrome/util': ['cr'],
'intervals': ['interval'],
'graphs': ['graph'],
'slices': ['slice', 'thread_slice', 'process_slice'],
'linux': ['cpu', 'memory'],
'stacks': ['cpu_profiling'],
}

# Allows for nonstandard object names.
OBJECT_NAME_ALLOWLIST = {
'graphs/partition.sql': ['tree_structural_partition_by_group'],
}


# Given a regex pattern and a string to match against, returns all the
# matching positions. Specifically, it returns a dictionary from the line
Expand Down
48 changes: 24 additions & 24 deletions python/test/stdlib_unittest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_custom_module_prefix(self):
-- Comment
CREATE PERFETTO TABLE cr_table(
-- Column.
x INT
x LONG
) AS
SELECT 1;
'''.strip())
Expand All @@ -42,7 +42,7 @@ def test_custom_module_prefix(self):
self.assertEqual(fn.name, 'cr_table')
self.assertEqual(fn.desc, 'Comment')
self.assertEqual(fn.cols, {
'x': Arg('INT', 'Column.'),
'x': Arg('LONG', 'Column.'),
})

# Checks that when custom prefixes (cr for chrome/util) are present,
Expand All @@ -53,7 +53,7 @@ def test_custom_module_prefix_full_module_name(self):
-- Comment
CREATE PERFETTO TABLE chrome_table(
-- Column.
x INT
x LONG
) AS
SELECT 1;
'''.strip())
Expand All @@ -63,7 +63,7 @@ def test_custom_module_prefix_full_module_name(self):
self.assertEqual(fn.name, 'chrome_table')
self.assertEqual(fn.desc, 'Comment')
self.assertEqual(fn.cols, {
'x': Arg('INT', 'Column.'),
'x': Arg('LONG', 'Column.'),
})

# Checks that when custom prefixes (cr for chrome/util) are present,
Expand All @@ -74,7 +74,7 @@ def test_custom_module_prefix_incorrect(self):
-- Comment
CREATE PERFETTO TABLE foo_table(
-- Column.
x INT
x LONG
) AS
SELECT 1;
'''.strip())
Expand All @@ -90,7 +90,7 @@ def test_custom_module_prefix_does_not_apply_outside(self):
-- Comment
CREATE PERFETTO TABLE cr_table(
-- Column.
x INT
x LONG
) AS
SELECT 1;
'''.strip())
Expand Down Expand Up @@ -155,7 +155,7 @@ def test_table_with_schema(self):
-- Table comment.
CREATE PERFETTO TABLE foo_table(
-- Id of slice.
id INT
id LONG
) AS
SELECT 1 as id;
'''.strip())
Expand All @@ -166,7 +166,7 @@ def test_table_with_schema(self):
self.assertEqual(table.desc, 'Table comment.')
self.assertEqual(table.type, 'TABLE')
self.assertEqual(table.cols, {
'id': Arg('INT', 'Id of slice.'),
'id': Arg('LONG', 'Id of slice.'),
})

def test_perfetto_view_with_schema(self):
Expand All @@ -175,7 +175,7 @@ def test_perfetto_view_with_schema(self):
-- View comment.
CREATE PERFETTO VIEW foo_table(
-- Foo.
foo INT,
foo LONG,
-- Bar.
bar STRING
) AS
Expand All @@ -188,7 +188,7 @@ def test_perfetto_view_with_schema(self):
self.assertEqual(table.desc, 'View comment.')
self.assertEqual(table.type, 'VIEW')
self.assertEqual(table.cols, {
'foo': Arg('INT', 'Foo.'),
'foo': Arg('LONG', 'Foo.'),
'bar': Arg('STRING', 'Bar.'),
})

Expand All @@ -198,7 +198,7 @@ def test_function_with_new_style_docs(self):
-- Function foo.
CREATE PERFETTO FUNCTION foo_fn(
-- Utid of thread.
utid INT,
utid LONG,
-- String name.
name STRING)
-- Exists.
Expand All @@ -213,7 +213,7 @@ def test_function_with_new_style_docs(self):
self.assertEqual(fn.desc, 'Function foo.')
self.assertEqual(
fn.args, {
'utid': Arg('INT', 'Utid of thread.'),
'utid': Arg('LONG', 'Utid of thread.'),
'name': Arg('STRING', 'String name.'),
})
self.assertEqual(fn.return_type, 'BOOL')
Expand All @@ -225,11 +225,11 @@ def test_function_returns_table_with_new_style_docs(self):
-- Function foo.
CREATE PERFETTO FUNCTION foo_fn(
-- Utid of thread.
utid INT)
utid LONG)
-- Impl comment.
RETURNS TABLE(
-- Count.
count INT
count LONG
)
AS
SELECT 1;
Expand All @@ -240,10 +240,10 @@ def test_function_returns_table_with_new_style_docs(self):
self.assertEqual(fn.name, 'foo_fn')
self.assertEqual(fn.desc, 'Function foo.')
self.assertEqual(fn.args, {
'utid': Arg('INT', 'Utid of thread.'),
'utid': Arg('LONG', 'Utid of thread.'),
})
self.assertEqual(fn.cols, {
'count': Arg('INT', 'Count.'),
'count': Arg('LONG', 'Count.'),
})

def test_function_with_new_style_docs_multiline_comment(self):
Expand All @@ -255,7 +255,7 @@ def test_function_with_new_style_docs_multiline_comment(self):
-- line
--
-- comment.
arg INT)
arg LONG)
-- Exists.
RETURNS BOOL
AS
Expand All @@ -267,7 +267,7 @@ def test_function_with_new_style_docs_multiline_comment(self):
self.assertEqual(fn.name, 'foo_fn')
self.assertEqual(fn.desc, 'Function foo.')
self.assertEqual(fn.args, {
'arg': Arg('INT', 'Multi line comment.'),
'arg': Arg('LONG', 'Multi line comment.'),
})
self.assertEqual(fn.return_type, 'BOOL')
self.assertEqual(fn.return_desc, 'Exists.')
Expand All @@ -278,7 +278,7 @@ def test_function_with_multiline_return_comment(self):
-- Function foo.
CREATE PERFETTO FUNCTION foo_fn(
-- Arg
arg INT)
arg LONG)
-- Multi
-- line
-- return
Expand All @@ -293,7 +293,7 @@ def test_function_with_multiline_return_comment(self):
self.assertEqual(fn.name, 'foo_fn')
self.assertEqual(fn.desc, 'Function foo.')
self.assertEqual(fn.args, {
'arg': Arg('INT', 'Arg'),
'arg': Arg('LONG', 'Arg'),
})
self.assertEqual(fn.return_type, 'BOOL')
self.assertEqual(fn.return_desc, 'Multi line return comment.')
Expand All @@ -304,7 +304,7 @@ def test_create_or_replace_table_banned(self):
-- Table.
CREATE OR REPLACE PERFETTO TABLE foo(
-- Column.
x INT
x LONG
)
AS
SELECT 1;
Expand All @@ -319,7 +319,7 @@ def test_create_or_replace_view_banned(self):
-- Table.
CREATE OR REPLACE PERFETTO VIEW foo(
-- Column.
x INT
x LONG
)
AS
SELECT 1;
Expand Down Expand Up @@ -347,7 +347,7 @@ def test_function_with_new_style_docs_with_parenthesis(self):
-- Function foo.
CREATE PERFETTO FUNCTION foo_fn(
-- Utid of thread (important).
utid INT)
utid LONG)
-- Exists.
RETURNS BOOL
AS
Expand All @@ -359,7 +359,7 @@ def test_function_with_new_style_docs_with_parenthesis(self):
self.assertEqual(fn.name, 'foo_fn')
self.assertEqual(fn.desc, 'Function foo.')
self.assertEqual(fn.args, {
'utid': Arg('INT', 'Utid of thread (important).'),
'utid': Arg('LONG', 'Utid of thread (important).'),
})
self.assertEqual(fn.return_type, 'BOOL')
self.assertEqual(fn.return_desc, 'Exists.')
Expand Down
Loading

0 comments on commit 834aa3b

Please sign in to comment.