Skip to content

Enable experimental non-null #181

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.

Sign up for GitHub

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

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Enable experimental non-null #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
8efcf9e
Make src/vsgi/vsgi-request.vala non-null compliant
Bob131 May 30, 2016
8037fbf
Make src/vsgi/vsgi-response.vala non-null compliant
Bob131 May 30, 2016
f1070a2
Make src/vsgi/vsgi-server.vala non-null compliant
Bob131 May 30, 2016
c1939a7
Make src/vsgi/vsgi-socket-listener-server.vala non-null compliant
Bob131 May 30, 2016
89467cd
Make src/vsgi-http/vsgi-http.vala non-null compliant
Bob131 May 30, 2016
623bd0f
Make src/vsgi-cgi/vsgi-cgi.vala non-null compliant
Bob131 May 30, 2016
845f528
Make src/vsgi-scgi/vsgi-scgi.vala non-null compliant
Bob131 May 30, 2016
f1b0ab9
Make src/vsgi-fastcgi/vsgi-fastcgi.vala non-null compliant
Bob131 May 30, 2016
22d6796
Make src/valum/valum-context.vala non-null compliant
Bob131 May 30, 2016
153e1e7
Make src/valum/valum-regex-route.vala non-null compliant
Bob131 May 30, 2016
5447410
Make src/valum/valum-rule-route.vala non-null compliant
Bob131 May 30, 2016
ea965aa
Make src/valum/valum-server-sent-events.vala non-null compliant
Bob131 May 30, 2016
fad5671
Make src/valum/valum-basepath.vala non-null compliant
Bob131 May 30, 2016
005d40e
Make src/valum/valum-decode.vala non-null compliant
Bob131 May 30, 2016
ca384da
Make src/valum/valum-content-negotiation.vala non-null compliant
Bob131 May 30, 2016
695f3a6
Enable non-null valac flag
Bob131 May 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ add_global_arguments(['-Wall',
language: 'c')

add_global_arguments(['--enable-experimental',
'--enable-experimental-non-null',
'--enable-deprecated',
'--fatal-warnings'],
language: 'vala')
Expand Down
8 changes: 4 additions & 4 deletions src/valum/valum-basepath.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ namespace Valum {
try {
return forward (req, res, () => {
req.uri.set_path (original_path);
var location = res.headers.get_one ("Location");
if (!res.head_written && location != null && location[0] == '/')
string? location = res.headers.get_one ("Location");
if (!res.head_written && location != null && ((!) location)[0] == '/')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to get rid of var here?

res.headers.replace ("Location", path + location);
return next ();
}, context);
Expand All @@ -64,8 +64,8 @@ namespace Valum {
throw err;
} finally {
req.uri.set_path (original_path);
var location = res.headers.get_one ("Location");
if (!res.head_written && location != null && location[0] == '/')
string? location = res.headers.get_one ("Location");
if (!res.head_written && location != null && ((!) location)[0] == '/')
res.headers.replace ("Location", path + location);
}
} else {
Expand Down
14 changes: 7 additions & 7 deletions src/valum/valum-content-negotiation.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ namespace Valum.ContentNegotiation {
if (param_end == -1)
param_end = header.length;

var @params = Soup.header_parse_semi_param_list (header[param_start:param_end]);
HashTable<string, string?> @params = Soup.header_parse_semi_param_list (header[param_start:param_end]);

double qvalue;
if (double.try_parse (@params["q"] ?? "1", out qvalue)) {
if (double.try_parse ((!) (@params["q"] ?? "1"), out qvalue)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should try something like follow to avoid the (!):

string q = @params["q"] ?? "1";
if (double.try_parse (q, out qvalue) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

../src/valum/valum-content-negotiation.vala:52.10-52.32: error: Assignment: Cannot convert from `string?' to `string'
                string q = @params["q"] ?? "1";
                       ^^^^^^^^^^^^^^^^^^^^^^^

return qvalue.clamp (0, 1);
}

Expand Down Expand Up @@ -83,7 +83,7 @@ namespace Valum.ContentNegotiation {
EqualFunc<string> match = (EqualFunc<string>) Soup.str_case_equal) {
var _expectations = Soup.header_parse_quality_list (expectations, null);
return (req, res, next, ctx) => {
var header = req.headers.get_list (header_name);
string? header = req.headers.get_list (header_name);
if (_expectations.length () == 0)
throw new ClientError.NOT_ACCEPTABLE ("'%s' cannot be satisfied: nothing is expected", header_name);
if (header == null) {
Expand All @@ -92,9 +92,9 @@ namespace Valum.ContentNegotiation {

string? best_expectation = null;
double best_qvalue = 0;
foreach (var accepted in Soup.header_parse_quality_list (header, null)) {
foreach (var accepted in Soup.header_parse_quality_list ((!) header, null)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this (added a if branch`) in d463968 to support unacceptable expectations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I rebase from master then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could help, but it's easy to merge. You should squash all the fixes in a single commit. This can go on forever ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you prefer to merge a single commit? I went with a bunch of smaller commits so that the commit messages could accurately describe the changes; I thought it might make the review process a bit easier. But I'm certainly not averse to squashing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple commits is fine for reviewing; I'll squash on the merge.

It's almost always a matter of history readability and ability to revert changes efficiently. If all the fixes are spread among dozens of commits, it makes both criteria hard to satisfy.

foreach (var expectation in _expectations) {
var current_qvalue = _qvalue_for_param (header, accepted) * _qvalue_for_param (expectations, expectation);
var current_qvalue = _qvalue_for_param ((!) header, accepted) * _qvalue_for_param (expectations, expectation);
if (match (accepted, expectation) && current_qvalue > best_qvalue) {
best_expectation = expectation;
best_qvalue = current_qvalue;
Expand All @@ -103,7 +103,7 @@ namespace Valum.ContentNegotiation {
}

if (best_expectation != null) {
return forward (req, res, next, ctx, best_expectation);
return forward (req, res, next, ctx, (!) best_expectation);
}

throw new ClientError.NOT_ACCEPTABLE ("'%s' is not satisfiable by any of '%s'.",
Expand Down Expand Up @@ -152,7 +152,7 @@ namespace Valum.ContentNegotiation {
owned NegotiateCallback forward) {
return negotiate ("Accept-Charset", charsets, (req, res, next, ctx, charset) => {
HashTable<string, string> @params;
var content_type = res.headers.get_content_type (out @params) ?? "application/octet-stream";
var content_type = (!) ((string?) res.headers.get_content_type (out @params) ?? "application/octet-stream");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure that if you introduce the type, it will not warn for nullable:

string content_type = ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

../src/valum/valum-content-negotiation.vala:158.11-158.109: error: Assignment: Cannot convert from `string?' to `string'
                        string content_type = ((string?) res.headers.get_content_type (out @params) ?? "application/octet-stream");
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like coalesed expressions nullability is just not infered.

I might try to patch that in valac, it does not look too difficult.

@params["charset"] = charset;
res.headers.set_content_type (content_type, @params);
return forward (req, res, next, ctx, charset);
Expand Down
4 changes: 2 additions & 2 deletions src/valum/valum-context.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public class Valum.Context : Object {
* @since 0.3
*/
public new Value? @get (string key) {
return states[key] ?? (parent == null ? null : parent[key]);
return states[key] ?? (parent == null ? null : ((!) parent)[key]);
}

/**
Expand All @@ -78,6 +78,6 @@ public class Valum.Context : Object {
* @since 0.3
*/
public bool contains (string key) {
return states.contains (key) || (parent != null && parent.contains (key));
return states.contains (key) || (parent != null && ((!) parent).contains (key));
}
}
17 changes: 9 additions & 8 deletions src/valum/valum-decode.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ namespace Valum {
*/
public HandlerCallback decode (DecodeFlags flags = DecodeFlags.NONE) {
return (req, res, next, ctx) => {
var encodings = Soup.header_parse_list (req.headers.get_list ("Content-Encoding") ?? "");
var encodings = Soup.header_parse_list ((!) ((string?) req.headers.get_list ("Content-Encoding") ?? ""));

// decode is in the opposite order of application
encodings.reverse ();

req.headers.remove ("Content-Encoding");

for (unowned SList<string> encoding = encodings; encoding != null; encoding = encoding.next) {
switch (encoding.data.down ()) {
for (unowned SList<string>? encoding = encodings; encoding != null; encoding = ((!) encoding).next) {
switch (((!) encoding).data.down ()) {
case "gzip":
case "x-gzip":
req.convert (new ZlibDecompressor (ZlibCompressorFormat.GZIP));
Expand All @@ -63,17 +63,18 @@ namespace Valum {
case "identity":
// nothing to do, let's take a break ;)
break;
default:
// reapply remaining encodings
encoding.reverse ();
foreach (var remaining in encoding) {
default: // reapply remaining encodings
// define new var since vala-0.26 chokes on some syntax
unowned SList<string> enc = (!) encoding;
enc.reverse ();
foreach (var remaining in enc) {
req.headers.append ("Content-Encoding", remaining);
}
if (DecodeFlags.FORWARD_REMAINING_ENCODINGS in flags) {
return next ();
} else {
throw new ServerError.NOT_IMPLEMENTED ("The '%s' encoding is not supported.",
encoding.data);
enc.data);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/valum/valum-regex-route.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ namespace Valum {
if (/\(\?<(\w+)>.+?\)/.match (regex.get_pattern (), 0, out capture_match_info)) {
try {
do {
captures.append (capture_match_info.fetch (1));
if (capture_match_info.fetch (1) != null)
captures.append ((!) capture_match_info.fetch (1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the capture is null, that would be a nasty bug to figure out. There's an error below you should warn with critical.

} while (capture_match_info.next ());
} catch (RegexError err) {
// ...
Expand Down
4 changes: 2 additions & 2 deletions src/valum/valum-rule-route.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ namespace Valum {
if (types == null) {
pattern.append_printf ("(?<%s>\\w+)", key);
} else {
if (!types.contains (type))
if (!((!) types).contains (type))
throw new RegexError.COMPILE ("using an undefined type %s", type);

pattern.append_printf ("(?<%s>%s)", key, types[type].get_pattern ());
pattern.append_printf ("(?<%s>%s)", key, ((!) types)[type].get_pattern ());
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/valum/valum-server-sent-events.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ namespace Valum.ServerSentEvents {
var message = new StringBuilder ();

if (event != null)
message.append_printf ("event: %s\n", event);
message.append_printf ("event: %s\n", (!) event);

if (id != null)
message.append_printf ("id: %s\n", id);
message.append_printf ("id: %s\n", (!) id);

if (retry != null)
message.append_printf ("retry: %d\n", (int) (retry / 1000));
Expand Down
40 changes: 20 additions & 20 deletions src/vsgi-cgi/vsgi-cgi.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ namespace VSGI.CGI {

public override string gateway_interface {
owned get {
return Environ.get_variable (environment, "GATEWAY_INTERFACE") ?? "CGI/1.1";
return (!) (Environ.get_variable (environment, "GATEWAY_INTERFACE") ?? "CGI/1.1");
}
}

public override string method {
owned get {
return Environ.get_variable (environment, "REQUEST_METHOD") ?? "GET";
return (!) (Environ.get_variable (environment, "REQUEST_METHOD") ?? "GET");
}
}

Expand Down Expand Up @@ -90,7 +90,7 @@ namespace VSGI.CGI {

var https = Environ.get_variable (environment, "HTTPS");
var path_translated = Environ.get_variable (environment, "PATH_TRANSLATED");
if (https != null && https.length > 0 || path_translated != null && path_translated.has_prefix ("https://"))
if (https != null && ((!) https).length > 0 || path_translated != null && ((!) path_translated).has_prefix ("https://"))
this._uri.set_scheme ("https");
else
this._uri.set_scheme ("http");
Expand All @@ -100,37 +100,37 @@ namespace VSGI.CGI {

var port = Environ.get_variable (environment, "SERVER_PORT");
if (port != null)
this._uri.set_port (int.parse (port));
this._uri.set_port (int.parse ((!) port));

var path_info = Environ.get_variable (environment, "PATH_INFO");
var request_uri = Environ.get_variable (environment, "REQUEST_URI");
if (path_info != null && path_info.length > 0)
this._uri.set_path (path_info);
else if (request_uri != null && request_uri.length > 0)
this._uri.set_path (request_uri.split ("?", 2)[0]); // strip the query
if (path_info != null && ((!) path_info).length > 0)
this._uri.set_path ((!) path_info);
else if (request_uri != null && ((!) request_uri).length > 0)
this._uri.set_path (((!) request_uri).split ("?", 2)[0]); // strip the query
else
this._uri.set_path ("/");

// raw & parsed HTTP query
var query_string = Environ.get_variable (environment, "QUERY_STRING");
if (query_string != null && query_string.length > 0) {
this._uri.set_query (query_string);
this._query = Form.decode (query_string);
} else if (path_translated != null && "?" in path_translated) {
this._uri.set_query (path_translated.split ("?", 2)[1]);
this._query = Form.decode (path_translated.split ("?", 2)[1]);
} else if (request_uri != null && "?" in request_uri) {
this._uri.set_query (request_uri.split ("?", 2)[1]);
this._query = Form.decode (request_uri.split ("?", 2)[1]);
if (query_string != null && ((!) query_string).length > 0) {
this._uri.set_query ((!) query_string);
this._query = Form.decode ((!) query_string);
} else if (path_translated != null && "?" in (!) path_translated) {
this._uri.set_query (((!) path_translated).split ("?", 2)[1]);
this._query = Form.decode (((!) path_translated).split ("?", 2)[1]);
} else if (request_uri != null && "?" in (!) request_uri) {
this._uri.set_query (((!) request_uri).split ("?", 2)[1]);
this._query = Form.decode (((!) request_uri).split ("?", 2)[1]);
}

var content_type = Environ.get_variable (environment, "CONTENT_TYPE") ?? "application/octet-stream";
var content_type = (!) (Environ.get_variable (environment, "CONTENT_TYPE") ?? "application/octet-stream");
var @params = Soup.header_parse_param_list (content_type);
headers.set_content_type (content_type.split (";", 2)[0], @params);

//
int64 content_length;
if (int64.try_parse (Environ.get_variable (environment, "CONTENT_LENGTH") ?? "0",
if (int64.try_parse ((!) (Environ.get_variable (environment, "CONTENT_LENGTH") ?? "0"),
out content_length)) {
headers.set_content_length (content_length);
}
Expand Down Expand Up @@ -164,7 +164,7 @@ namespace VSGI.CGI {
protected override uint8[]? build_head () {
var head = new StringBuilder ();

head.append_printf ("Status: %u %s\r\n", status, reason_phrase ?? Status.get_phrase (status));
head.append_printf ("Status: %u %s\r\n", status, (!) (reason_phrase ?? Status.get_phrase (status)));

this.headers.foreach ((k, v) => {
head.append_printf ("%s: %s\r\n", k, v);
Expand Down
20 changes: 9 additions & 11 deletions src/vsgi-fastcgi/vsgi-fastcgi.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,11 @@ namespace VSGI.FastCGI {

construct {
#if GIO_2_40
const OptionEntry[] options = {
{"socket", 's', 0, OptionArg.FILENAME, null, "Listen to the provided UNIX domain socket (or named pipe for WinNT)"},
{"port", 'p', 0, OptionArg.INT, null, "Listen to the provided TCP port"},
{"file-descriptor", 'f', 0, OptionArg.INT, null, "Listen to the provided file descriptor", "0"},
{"backlog", 'b', 0, OptionArg.INT, null, "Listen queue depth used in the listen() call", "10"},
{null}
};
var options = new OptionEntry[4];
options[0] = {"socket", 's', 0, OptionArg.FILENAME, null, "Listen to the provided UNIX domain socket (or named pipe for WinNT)"};
options[1] = {"port", 'p', 0, OptionArg.INT, null, "Listen to the provided TCP port"};
options[2] = {"file-descriptor", 'f', 0, OptionArg.INT, null, "Listen to the provided file descriptor", "0"};
options[3] = {"backlog", 'b', 0, OptionArg.INT, null, "Listen queue depth used in the listen() call", "10"};
this.add_main_option_entries (options);
#endif

Expand All @@ -204,12 +202,12 @@ namespace VSGI.FastCGI {
}

public override void listen (Variant options) throws Error {
var backlog = options.lookup_value ("backlog", VariantType.INT32) ?? new Variant.@int32 (10);
var backlog = (!) (options.lookup_value ("backlog", VariantType.INT32) ?? new Variant.@int32 (10));

var fd = global::FastCGI.LISTENSOCK_FILENO;

if (options.lookup_value ("socket", VariantType.BYTESTRING) != null) {
var socket_path = options.lookup_value ("socket", VariantType.BYTESTRING).get_bytestring ();
var socket_path = ((!) options.lookup_value ("socket", VariantType.BYTESTRING)).get_bytestring ();

fd = global::FastCGI.open_socket (socket_path, backlog.get_int32 ());

Expand All @@ -221,7 +219,7 @@ namespace VSGI.FastCGI {
}

else if (options.lookup_value ("port", VariantType.INT32) != null) {
var port = (options.lookup_value ("port", VariantType.INT32).get_int32 ());
var port = ((!) options.lookup_value ("port", VariantType.INT32)).get_int32 ();

fd = global::FastCGI.open_socket (":%d".printf (port), backlog.get_int32 ());

Expand All @@ -233,7 +231,7 @@ namespace VSGI.FastCGI {
}

else if (options.lookup_value ("file-descriptor", VariantType.INT32) != null) {
fd = options.lookup_value ("file-descriptor", VariantType.INT32).get_int32 ();
fd = ((!) options.lookup_value ("file-descriptor", VariantType.INT32)).get_int32 ();
_uris.append (new Soup.URI ("fcgi+fd://%u/".printf (fd)));
}

Expand Down
Loading