Commit d3385bae authored by Philip Withnall's avatar Philip Withnall
Browse files

core: Nullability fixes

Almost all of these are just the necessary ‘(!)’ annotations to allow the
nullability check to pass. There were few, if any, actual bugs found by the
check (which either means folks is perfect, or Vala's nullability checking
is imperfect).

This brings us down from 296 nullability errors to just below 50.

The work was done by compiling folks with valac’s
--enable-experimental-non-null flag. We’re not ready to add the flag to
VALAFLAGS permanently yet, since this commit depends on various annotation
fixes in GLib (and similarly, the next one depends on several in EDS).
However, the fixes themselves should be valid without the flag.

This depends on (at least):
 • https://bugzilla.gnome.org/show_bug.cgi?id=666700https://bugzilla.gnome.org/show_bug.cgi?id=666699
parent 9136e590
......@@ -271,8 +271,8 @@ public abstract class Folks.AbstractFieldDetails<T> : Object
public virtual bool equal (AbstractFieldDetails<T> that)
{
return (this.get_type () == that.get_type ()) &&
this.values_equal<T> (that) &&
this.parameters_equal<T> (that);
this.values_equal (that) &&
this.parameters_equal (that);
}
/**
......
......@@ -29,7 +29,7 @@ using GLib;
*/
public class Folks.AvatarCache : Object
{
private static weak AvatarCache _instance = null; /* needs to be locked */
private static weak AvatarCache? _instance = null; /* needs to be locked */
private File _cache_directory;
/**
......@@ -64,14 +64,19 @@ public class Folks.AvatarCache : Object
{
lock (AvatarCache._instance)
{
var retval = AvatarCache._instance;
var _retval = AvatarCache._instance;
AvatarCache retval;
if (retval == null)
if (_retval == null)
{
/* use an intermediate variable to force a strong reference */
retval = new AvatarCache ();
AvatarCache._instance = retval;
}
else
{
retval = (!) _retval;
}
return retval;
}
......@@ -143,9 +148,9 @@ public class Folks.AvatarCache : Object
dest_avatar_stream =
yield dest_avatar_file.replace_async (null, false,
FileCreateFlags.PRIVATE);
yield dest_avatar_stream.splice_async (src_avatar_stream,
yield ((!) dest_avatar_stream).splice_async (src_avatar_stream,
OutputStreamSpliceFlags.NONE);
yield dest_avatar_stream.close_async ();
yield ((!) dest_avatar_stream).close_async ();
break;
}
......@@ -161,7 +166,7 @@ public class Folks.AvatarCache : Object
if (dest_avatar_stream != null)
{
yield dest_avatar_stream.close_async ();
yield ((!) dest_avatar_stream).close_async ();
}
throw e;
......
......@@ -48,7 +48,7 @@ public class Folks.BackendStore : Object {
private File _config_file;
private GLib.KeyFile _backends_key_file;
private HashMap<string,unowned Module> _modules;
private static weak BackendStore _instance;
private static weak BackendStore? _instance = null;
private bool _is_prepared = false;
private Debug _debug;
......@@ -116,16 +116,16 @@ public class Folks.BackendStore : Object {
*/
public static BackendStore dup ()
{
if (_instance == null)
if (BackendStore._instance == null)
{
/* use an intermediate variable to force a strong reference */
var new_instance = new BackendStore ();
_instance = new_instance;
BackendStore._instance = new_instance;
return new_instance;
}
return _instance;
return (!) BackendStore._instance;
}
private BackendStore ()
......@@ -168,10 +168,9 @@ public class Folks.BackendStore : Object {
/* Disconnect from the debug handler */
this._debug.print_status.disconnect (this._debug_print_status);
this._debug = null;
/* manually clear the singleton instance */
_instance = null;
BackendStore._instance = null;
}
private void _debug_print_status (Debug debug)
......@@ -201,7 +200,7 @@ public class Folks.BackendStore : Object {
foreach (var persona_store in backend.persona_stores.values)
{
string trust_level = null;
string? trust_level = null;
switch (persona_store.trust_level)
{
......@@ -284,8 +283,10 @@ public class Folks.BackendStore : Object {
yield this._backend_unload_if_needed (backend_existing);
}
var path = Environment.get_variable ("FOLKS_BACKEND_PATH");
if (path == null)
string? _path = Environment.get_variable ("FOLKS_BACKEND_PATH");
string path;
if (_path == null)
{
path = BuildConf.BACKEND_DIR;
......@@ -294,16 +295,17 @@ public class Folks.BackendStore : Object {
}
else
{
path = (!) _path;
debug ("Using environment variable FOLKS_BACKEND_PATH = " +
"'%s' to look for backends", path);
}
var modules = new HashMap<string, File?> ();
var modules = new HashMap<string, File> ();
var path_split = path.split (":");
foreach (unowned string subpath in path_split)
{
var file = File.new_for_path (subpath);
assert (file != null);
bool is_file;
bool is_dir;
......@@ -315,8 +317,13 @@ public class Folks.BackendStore : Object {
else if (is_dir)
{
var cur_modules = yield this._get_modules_from_dir (file);
foreach (var entry in cur_modules.entries)
modules.set (entry.key, entry.value);
if (cur_modules != null)
{
foreach (var entry in ((!) cur_modules).entries)
{
modules.set (entry.key, entry.value);
}
}
}
else
{
......@@ -369,12 +376,12 @@ public class Folks.BackendStore : Object {
if (!this._backend_is_enabled (backend.name))
{
var backend_existing = this._backend_hash.get (backend.name);
Backend? backend_existing = this._backend_hash.get (backend.name);
if (backend_existing != null)
{
try
{
yield backend_existing.unprepare ();
yield ((!) backend_existing).unprepare ();
}
catch (GLib.Error e)
{
......@@ -382,7 +389,7 @@ public class Folks.BackendStore : Object {
e.message);
}
this._prepared_backends.unset (backend_existing.name);
this._prepared_backends.unset (((!) backend_existing).name);
unloaded = true;
}
......@@ -399,11 +406,11 @@ public class Folks.BackendStore : Object {
public void add_backend (Backend backend)
{
/* Purge any other backend with the same name; re-add if enabled */
var backend_existing = this._backend_hash.get (backend.name);
Backend? backend_existing = this._backend_hash.get (backend.name);
if (backend_existing != null && backend_existing != backend)
{
backend_existing.unprepare ();
this._prepared_backends.unset (backend_existing.name);
((!) backend_existing).unprepare ();
this._prepared_backends.unset (((!) backend_existing).name);
}
this._debug._register_domain (backend.name);
......@@ -566,17 +573,26 @@ public class Folks.BackendStore : Object {
* aliases */
var is_symlink = info.get_is_symlink ();
string mime = ContentType.get_mime_type (content_type);
string? mime = ContentType.get_mime_type (content_type);
if (file_type == FileType.DIRECTORY)
{
var modules = yield this._get_modules_from_dir (file);
foreach (var entry in modules.entries)
modules_final.set (entry.key, entry.value);
if (modules != null)
{
foreach (var entry in ((!) modules).entries)
{
modules_final.set (entry.key, entry.value);
}
}
}
else if (mime == "application/x-sharedlib" && !is_symlink)
{
modules_final.set (file.get_path (), file);
var path = file.get_path ();
if (path != null)
{
modules_final.set ((!) path, file);
}
}
else if (mime == null)
{
......@@ -595,13 +611,18 @@ public class Folks.BackendStore : Object {
private void _load_module_from_file (File file)
{
var file_path = file.get_path ();
var _file_path = file.get_path ();
if (_file_path == null)
{
return;
}
var file_path = (!) _file_path;
if (this._modules.has_key (file_path))
return;
Module module = Module.open (file_path, ModuleFlags.BIND_LOCAL);
if (module == null)
var _module = Module.open (file_path, ModuleFlags.BIND_LOCAL);
if (_module == null)
{
/* Translators: the first parameter is a filename and the second is an
* error message. */
......@@ -610,6 +631,7 @@ public class Folks.BackendStore : Object {
return;
}
unowned Module module = (!) _module;
void* function;
......@@ -681,7 +703,7 @@ public class Folks.BackendStore : Object {
private async void _load_disabled_backend_names ()
{
File file;
unowned string path = Environment.get_variable (
unowned string? path = Environment.get_variable (
"FOLKS_BACKEND_STORE_KEY_FILE_PATH");
if (path == null)
{
......@@ -695,10 +717,10 @@ public class Folks.BackendStore : Object {
}
else
{
file = File.new_for_path (path);
file = File.new_for_path ((!) path);
debug ("Using environment variable " +
"FOLKS_BACKEND_STORE_KEY_FILE_PATH = '%s' to load the backends " +
"key file.", path);
"key file.", (!) path);
}
this._config_file = file;
......
......@@ -48,7 +48,8 @@ public class Folks.Debug : Object
KEY_FILE_BACKEND = 1 << 2
}
private static weak Debug _instance; /* needs to be locked when accessed */
/* Needs to be locked when accessed: */
private static weak Debug? _instance = null;
private HashSet<string> _domains; /* needs to be locked when accessed */
private bool _all = false; /* needs _domains to be locked when accessed */
......@@ -196,14 +197,19 @@ public class Folks.Debug : Object
{
lock (Debug._instance)
{
var retval = Debug._instance;
Debug? _retval = Debug._instance;
Debug retval;
if (retval == null)
if (_retval == null)
{
/* use an intermediate variable to force a strong reference */
retval = new Debug ();
Debug._instance = retval;
}
else
{
retval = (!) _retval;
}
return retval;
}
......@@ -234,7 +240,7 @@ public class Folks.Debug : Object
if (debug_flags != null && debug_flags != "")
{
var domains_split = debug_flags.split (",");
var domains_split = ((!) debug_flags).split (",");
foreach (var domain in domains_split)
{
var domain_lower = domain.down ();
......@@ -283,7 +289,7 @@ public class Folks.Debug : Object
}
private void _set_handler (
string? domain,
string domain,
LogLevelFlags flags,
LogFunc log_func)
{
......@@ -429,7 +435,7 @@ public class Folks.Debug : Object
return "(null)";
}
return input;
return (!) input;
}
struct KeyValuePair
......@@ -465,11 +471,12 @@ public class Folks.Debug : Object
* purposes */
while (true)
{
string? key = valist.arg ();
if (key == null)
string? _key = valist.arg ();
if (_key == null)
{
break;
}
var key = (!) _key;
string? val = valist.arg ();
......
......@@ -57,7 +57,7 @@ public class Folks.EmailFieldDetails : AbstractFieldDetails<string>
this.value = value;
if (parameters != null)
this.parameters = parameters;
this.parameters = (!) parameters;
}
/**
......@@ -67,7 +67,7 @@ public class Folks.EmailFieldDetails : AbstractFieldDetails<string>
*/
public override bool equal (AbstractFieldDetails<string> that)
{
return base.equal<string> (that);
return base.equal (that);
}
/**
......
......@@ -66,7 +66,7 @@ public class Folks.ImFieldDetails : AbstractFieldDetails<string>
this.value = value;
if (parameters != null)
this.parameters = parameters;
this.parameters = (!) parameters;
}
/**
......@@ -76,7 +76,7 @@ public class Folks.ImFieldDetails : AbstractFieldDetails<string>
*/
public override bool equal (AbstractFieldDetails<string> that)
{
return base.equal<string> (that);
return base.equal (that);
}
/**
......@@ -179,7 +179,7 @@ public interface Folks.ImDetails : Object
im_address);
}
string resource = null;
string? resource = null;
if (parts.length == 2)
resource = parts[1];
......@@ -193,20 +193,20 @@ public interface Folks.ImDetails : Object
im_address);
}
string node, domain;
string? node, _domain;
if (parts.length == 2)
{
node = parts[0];
domain = parts[1];
_domain = parts[1];
}
else
{
node = null;
domain = parts[0];
_domain = parts[0];
}
if ((node != null && node == "") ||
(domain == null || domain == "") ||
(_domain == null || _domain == "") ||
(resource != null && resource == ""))
{
throw new ImDetailsError.INVALID_IM_ADDRESS (
......@@ -215,24 +215,24 @@ public interface Folks.ImDetails : Object
im_address);
}
domain = domain.down ();
string domain = ((!) _domain).down ();
if (node != null)
node = node.down ();
node = ((!) node).down ();
/* Build a new JID */
string normalised = null;
string? normalised = null;
if (node != null && resource != null)
{
normalised = "%s@%s/%s".printf (node, domain, resource);
normalised = "%s@%s/%s".printf ((!) node, domain, (!) resource);
}
else if (node != null)
{
normalised = "%s@%s".printf (node, domain);
normalised = "%s@%s".printf ((!) node, domain);
}
else if (resource != null)
{
normalised = "%s/%s".printf (domain, resource);
normalised = "%s/%s".printf (domain, (!) resource);
}
else
{
......@@ -242,7 +242,7 @@ public interface Folks.ImDetails : Object
im_address);
}
return normalised.normalize (-1, NormalizeMode.NFKC);
return ((!) normalised).normalize (-1, NormalizeMode.NFKC);
}
else
{
......
......@@ -185,7 +185,7 @@ public class Folks.IndividualAggregator : Object
*
* @since 0.3.0
*/
public Individual user { get; private set; }
public Individual? user { get; private set; }
/**
* Emitted when one or more {@link Individual}s are added to or removed from
......@@ -303,7 +303,7 @@ public class Folks.IndividualAggregator : Object
if (store_config_ids != null)
{
debug ("Setting primary store IDs from environment variable.");
this._configure_primary_store (store_config_ids);
this._configure_primary_store ((!) store_config_ids);
}
else
{
......@@ -325,8 +325,13 @@ public class Folks.IndividualAggregator : Object
GConf.Value? val = client.get (this._FOLKS_CONFIG_KEY);
if (val != null)
{
debug ("Setting primary store IDs from GConf.");
this._configure_primary_store (val.get_string ());
string? val_str = ((!) val).get_string ();
if (val_str != null)
{
debug ("Setting primary store IDs from GConf.");
this._configure_primary_store ((!) val_str);
}
}
}
catch (GLib.Error e)
......@@ -341,7 +346,7 @@ public class Folks.IndividualAggregator : Object
var disable_linking = Environment.get_variable ("FOLKS_DISABLE_LINKING");
if (disable_linking != null)
disable_linking = disable_linking.strip ().down ();
disable_linking = ((!) disable_linking).strip ().down ();
this._linking_enabled = (disable_linking == null ||
disable_linking == "no" || disable_linking == "0");
......@@ -354,7 +359,6 @@ public class Folks.IndividualAggregator : Object
{
this._backend_store.backend_available.disconnect (
this._backend_available_cb);
this._backend_store = null;
this._debug.print_status.disconnect (this._debug_print_status);
}
......@@ -403,7 +407,7 @@ public class Folks.IndividualAggregator : Object
foreach (var individual in this.individuals.values)
{
string trust_level = null;
string? trust_level = null;
switch (individual.trust_level)
{
......@@ -569,22 +573,34 @@ public class Folks.IndividualAggregator : Object
for (var i = 0; i < individuals.length; i++)
{
var a = individuals[i];
var matches_a = matches.get (a);
if (matches_a == null)
HashMap<Individual, MatchResult>? _matches_a = matches.get (a);
HashMap<Individual, MatchResult> matches_a;
if (_matches_a == null)
{
matches_a = new HashMap<Individual, MatchResult> ();
matches.set (a, matches_a);
}
else
{
matches_a = (!) _matches_a;
}
for (var f = i + 1; f < individuals.length; f++)
{
var b = individuals[f];
var matches_b = matches.get (b);
if (matches_b == null)
HashMap<Individual, MatchResult>? _matches_b = matches.get (b);
HashMap<Individual, MatchResult> matches_b;
if (_matches_b == null)
{
matches_b = new HashMap<Individual, MatchResult> ();
matches.set (b, matches_b);
}
else
{
matches_b = (!) _matches_b;
}
var result = matchObj.potential_match (a, b);
......@@ -664,16 +680,16 @@ public class Folks.IndividualAggregator : Object
var previous_store = this._primary_store;
this._primary_store = store;
this._primary_store.freeze_notify ();
store.freeze_notify ();
if (previous_store != null)
{
previous_store.freeze_notify ();
previous_store.is_primary_store = false;
((!) previous_store).freeze_notify ();
((!) previous_store).is_primary_store = false;
}
this._primary_store.is_primary_store = true;
store.is_primary_store = true;
if (previous_store != null)
previous_store.thaw_notify ();
this._primary_store.thaw_notify ();
((!) previous_store).thaw_notify ();
store.thaw_notify ();
this.notify_property ("primary-store");
}
......@@ -771,26 +787,26 @@ public class Folks.IndividualAggregator : Object
Persona? actor = null,
GroupDetails.ChangeReason reason = GroupDetails.ChangeReason.NONE)
{
var _added = added;
var _removed = removed;
var _changes = changes;
Set<Individual> _added;
Set<Individual> _removed;
MultiMap<Individual?, Individual?> _changes;
if ((added == null || added.size == 0) &&
(removed == null || removed.size == 0) &&
(changes == null || changes.size == 0))
if ((added == null || ((!) added).size == 0) &&
(removed == null || ((!) removed).size == 0) &&
(changes == null || ((!) changes).size == 0))
{
/* Don't bother emitting it if nothing's changed */
return;
}
else if (added == null)
{
_added = new HashSet<Individual> ();
}
else if (removed == null)
_added = (added != null) ? (!) added : new HashSet<Individual> ();
_removed = (removed != null) ? (!) removed : new HashSet<Individual> ();
if (changes != null)
{
_removed = new HashSet<Individual> ();
_changes = (!) changes;
}
else if (changes == null)
else
{
_changes = new HashMultiMap<Individual?, Individual?> ();
}
......@@ -806,14 +822,15 @@ public class Folks.IndividualAggregator : Object
foreach (var added_ind in _changes.get (removed_ind))
{
debug (" %s (%p) → %s (%p)",
(removed_ind != null) ? removed_ind.id : "", removed_ind,
(added_ind != null) ? added_ind